[3/4] qemu-config: Add -drive fd and opaque options

These new options can be used for passing drive file descriptors
on the command line, instead of using the file option to specify
a file name.

These new command line options mirror the existing add-fd QMP
command which allows an fd to be passed to QEMU via SCM_RIGHTS and
added to an fd set.  The opaque option is also available with
add-fd, and allows a free-form string to be stored in the fd set
along with the fd.

Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
 qemu-config.c   |  8 ++++++++
 qemu-options.hx | 15 +++++++++++++--
 2 files changed, 21 insertions(+), 2 deletions(-)

On Fri, Oct 5, 2012 at 6:07 PM, Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
> These new options can be used for passing drive file descriptors
> on the command line, instead of using the file option to specify
> a file name.
>
> These new command line options mirror the existing add-fd QMP
> command which allows an fd to be passed to QEMU via SCM_RIGHTS and
> added to an fd set.  The opaque option is also available with
> add-fd, and allows a free-form string to be stored in the fd set
> along with the fd.
>
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> ---
>  qemu-config.c   |  8 ++++++++
>  qemu-options.hx | 15 +++++++++++++--
>  2 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-config.c b/qemu-config.c
> index cd1ec21..91053dd 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -114,6 +114,14 @@ static QemuOptsList qemu_drive_opts = {
>              .name = "copy-on-read",
>              .type = QEMU_OPT_BOOL,
>              .help = "copy read data from backing file into image file",
> +        },{
> +            .name = "fd",
> +            .type = QEMU_OPT_NUMBER,
> +            .help = "disk image file descriptor",
> +        },{
> +            .name = "opaque",

'opaque' is not very descriptive and it's also not obvious (except
from the help text) that it's only interesting for file descriptors.
How about fd_name, fd_tag or fd_descr?

> +            .type = QEMU_OPT_STRING,
> +            .help = "free-form string used to describe fd",
>          },
>          { /* end of list */ }
>      },
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 7d97f96..513530f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -149,7 +149,7 @@ using @file{/dev/cdrom} as filename (@pxref{host_drives}).
>  ETEXI
>
>  DEF("drive", HAS_ARG, QEMU_OPTION_drive,
> -    "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
> +    "-drive [file=file|fd=fd[,opaque=o]][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
>      "       [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
>      "       [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n"
>      "       [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
> @@ -170,6 +170,12 @@ this drive. If the filename contains comma, you must double it
>
>  Special files such as iSCSI devices can be specified using protocol
>  specific URLs. See the section for "Device URL Syntax" for more information.
> +@item fd=@var{fd}
> +This option defines which disk image (@pxref{disk_images}) file descriptor to
> +use with this drive.
> +@item opaque=@var{opaque}
> +This option defines a free-form string that describes @var{fd}.  This is used
> +when storing @var{fd} in a file descriptor set.
>  @item if=@var{interface}
>  This option defines on which type on interface the drive is connected.
>  Available types are: ide, scsi, sd, mtd, floppy, pflash, virtio.
> @@ -257,12 +263,17 @@ qemu-system-i386 -drive file=file,index=2,media=disk
>  qemu-system-i386 -drive file=file,index=3,media=disk
>  @end example
>
> +You can open an image using a pre-opened file descriptor:
> +@example
> +qemu-system-i386 -drive fd=24,opaque="rdwr:/path/to/file",index=0,media=disk
> +@end example
> +
>  You can connect a CDROM to the slave of ide0:
>  @example
>  qemu-system-i386 -drive file=file,if=ide,index=1,media=cdrom
>  @end example
>
> -If you don't specify the "file=" argument, you define an empty drive:
> +If you don't specify the "file=" or "fd=" arguments, you define an empty drive:
>  @example
>  qemu-system-i386 -drive if=ide,index=1,media=cdrom
>  @end example
> --
> 1.7.11.4
>
>

On 10/05/2012 12:25 PM, Blue Swirl wrote:
> On Fri, Oct 5, 2012 at 6:07 PM, Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>> These new options can be used for passing drive file descriptors
>> on the command line, instead of using the file option to specify
>> a file name.
>>
>> These new command line options mirror the existing add-fd QMP
>> command which allows an fd to be passed to QEMU via SCM_RIGHTS and
>> added to an fd set.  The opaque option is also available with
>> add-fd, and allows a free-form string to be stored in the fd set
>> along with the fd.
>>

>> +            .name = "opaque",
> 
> 'opaque' is not very descriptive and it's also not obvious (except
> from the help text) that it's only interesting for file descriptors.
> How about fd_name, fd_tag or fd_descr?

Hmm, since opaque is per-fd in the existing monitor command, that means
my proposal needs a slight modification to:

 -fdset set=1,fd=24,opaque="rdonly",fd=25,opaque="rdwr"

or some other way where we can specify multiple fds and multiple opaque
strings per set.

At any rate, this just proves that we need to nail down the command line
implementation to something that is easy enough to use, before coding up
something that locks us in to bad design.

On 10/05/2012 02:30 PM, Eric Blake wrote:
> On 10/05/2012 12:25 PM, Blue Swirl wrote:
>> On Fri, Oct 5, 2012 at 6:07 PM, Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>>> These new options can be used for passing drive file descriptors
>>> on the command line, instead of using the file option to specify
>>> a file name.
>>>
>>> These new command line options mirror the existing add-fd QMP
>>> command which allows an fd to be passed to QEMU via SCM_RIGHTS and
>>> added to an fd set.  The opaque option is also available with
>>> add-fd, and allows a free-form string to be stored in the fd set
>>> along with the fd.
>>>
>
>>> +            .name = "opaque",
>>
>> 'opaque' is not very descriptive and it's also not obvious (except
>> from the help text) that it's only interesting for file descriptors.
>> How about fd_name, fd_tag or fd_descr?
>
> Hmm, since opaque is per-fd in the existing monitor command, that means
> my proposal needs a slight modification to:
>
>   -fdset set=1,fd=24,opaque="rdonly",fd=25,opaque="rdwr"

Yes, this makes more sense.  I'd like to mirror the add-fd QMP command 
as much as possible:

{ 'command': 'add-fd', 'data': {'*fdset-id': 'int', '*opaque': 'str'},
   'returns': 'AddfdInfo' }

So maybe we can make it:

-add-fd fd=24,fdset-id=1,opaque="rdonly" -add-fd 
fd=25,fdset-id=1,opaque="rdwr"

On 10/05/2012 02:25 PM, Blue Swirl wrote:
> On Fri, Oct 5, 2012 at 6:07 PM, Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>> These new options can be used for passing drive file descriptors
>> on the command line, instead of using the file option to specify
>> a file name.
>>
>> These new command line options mirror the existing add-fd QMP
>> command which allows an fd to be passed to QEMU via SCM_RIGHTS and
>> added to an fd set.  The opaque option is also available with
>> add-fd, and allows a free-form string to be stored in the fd set
>> along with the fd.
>>
>> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
>> ---
>>   qemu-config.c   |  8 ++++++++
>>   qemu-options.hx | 15 +++++++++++++--
>>   2 files changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/qemu-config.c b/qemu-config.c
>> index cd1ec21..91053dd 100644
>> --- a/qemu-config.c
>> +++ b/qemu-config.c
>> @@ -114,6 +114,14 @@ static QemuOptsList qemu_drive_opts = {
>>               .name = "copy-on-read",
>>               .type = QEMU_OPT_BOOL,
>>               .help = "copy read data from backing file into image file",
>> +        },{
>> +            .name = "fd",
>> +            .type = QEMU_OPT_NUMBER,
>> +            .help = "disk image file descriptor",
>> +        },{
>> +            .name = "opaque",
>
> 'opaque' is not very descriptive and it's also not obvious (except
> from the help text) that it's only interesting for file descriptors.
> How about fd_name, fd_tag or fd_descr?
>

I'd like to mirror the add-fd QMP command as much as possible:

{ 'command': 'add-fd', 'data': {'*fdset-id': 'int', '*opaque': 'str'},
   'returns': 'AddfdInfo' }

And I think use of opaque will make more sense if I don't merge these 
options in to -drive, and instead create a new command specifically for 
adding fds, like Eric is suggesting.

On 10/05/2012 12:44 PM, Corey Bryant wrote:
> Yes, this makes more sense.  I'd like to mirror the add-fd QMP command
> as much as possible:
> 
> { 'command': 'add-fd', 'data': {'*fdset-id': 'int', '*opaque': 'str'},
>   'returns': 'AddfdInfo' }
> 
> So maybe we can make it:
> 
> -add-fd fd=24,fdset-id=1,opaque="rdonly" -add-fd
> fd=25,fdset-id=1,opaque="rdwr"

Sounds better.  Note that while fdset-id is optional in QMP, I think it
needs to be mandatory on the CLI (you're telling qemu to use an
inherited fd, but unless you know what set that fd belongs to, you can't
then refer to that fd elsewhere on the command line, and unlike the QMP
command, there is no venue for QMP to tell you what set was autocreated).

Bike-shedding: I think the command line can be slightly shorter with:

-add-fd fd=24,set=1,opaque=...

with no real loss in information (that is, s/fdset-id/set/), since our
command lines are already quite long.  But going longhand to match QMP
doesn't hurt my feelings (libvirt will be automating this all anyway, so
I won't really be typing it by hand).

On 10/05/2012 02:51 PM, Eric Blake wrote:
> On 10/05/2012 12:44 PM, Corey Bryant wrote:
>> Yes, this makes more sense.  I'd like to mirror the add-fd QMP command
>> as much as possible:
>>
>> { 'command': 'add-fd', 'data': {'*fdset-id': 'int', '*opaque': 'str'},
>>    'returns': 'AddfdInfo' }
>>
>> So maybe we can make it:
>>
>> -add-fd fd=24,fdset-id=1,opaque="rdonly" -add-fd
>> fd=25,fdset-id=1,opaque="rdwr"
>
> Sounds better.  Note that while fdset-id is optional in QMP, I think it
> needs to be mandatory on the CLI (you're telling qemu to use an
> inherited fd, but unless you know what set that fd belongs to, you can't
> then refer to that fd elsewhere on the command line, and unlike the QMP
> command, there is no venue for QMP to tell you what set was autocreated).
>

I agree the fdset ID should be mandatory on the command line so we can 
link up other commands like '-drive file=/dev/fdset/nnn' where nnn is 
the fdset ID.

> Bike-shedding: I think the command line can be slightly shorter with:
>
> -add-fd fd=24,set=1,opaque=...
>

Sure, I have no problem with shortening it.