[libvirt] [PATCH] Add "io" option to virsh attach-disk sub-command.

[John Ferlan]

On 05/07/2017 05:44 PM, Gordon Messmer wrote:
> ---
>  tools/virsh-domain.c | 14 +++++++++++---
>  tools/virsh.pod      |  2 ++
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 

Well for some reason I cannot apply this patch using "git am -3"...

I think what you should do is rebase to the current git head and then
git send-email the patch again after applying some changes. When you
send it, be sure to note it's "v2"... I keep a template for that in a
file and use:

 git send-email --confirm=always --no-chain-reply-to --annotate \
    --cover-letter \
    --to=libvir-list at redhat.com master
   [ --subject-prefix="PATCH v#" ]

where --subject-prefix would be necessary for v2, v3, v4, etc. Also the
--cover-letter is only necessary for a 1 patch (so for this one it
wouldn't be).

You should take the text from your follow up emails and write a commit
message to describe the patch does. It should "look like" other patches
on the list - there's also the :

http://libvirt.org/hacking.html

to help you out.  Also be sure to run "make check syntax-check"

As for the patch itself...

> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 0d19d0e..d2a2a05 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -267,6 +267,10 @@ static const vshCmdOptDef opts_attach_disk[] = {
>       .type = VSH_OT_STRING,
>       .help = N_("cache mode of disk device")
>      },
> +    {.name = "io",
> +     .type = VSH_OT_STRING,
> +     .help = N_("io mode of disk device")
> +    },

I'd say this is not so much io mode, but rather io policy for thread
model used by the driver (or something like that).

>      {.name = "type",
>       .type = VSH_OT_STRING,
>       .help = N_("target device type")
> @@ -504,8 +508,9 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
>      virDomainPtr dom = NULL;
>      const char *source = NULL, *target = NULL, *driver = NULL,
>                  *subdriver = NULL, *type = NULL, *mode = NULL,
> -                *iothread = NULL, *cache = NULL, *serial = NULL,
> -                *straddr = NULL, *wwn = NULL, *targetbus = NULL;
> +                *iothread = NULL, *cache = NULL, *io = NULL,
> +                *serial = NULL, *straddr = NULL, *wwn = NULL,
> +                *targetbus = NULL;
>      struct DiskAddress diskAddr;
>      bool isFile = false, functionReturn = false;
>      int ret;
> @@ -537,6 +542,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
>          vshCommandOptStringReq(ctl, cmd, "mode", &mode) < 0 ||
>          vshCommandOptStringReq(ctl, cmd, "iothread", &iothread) < 0 ||
>          vshCommandOptStringReq(ctl, cmd, "cache", &cache) < 0 ||
> +        vshCommandOptStringReq(ctl, cmd, "io", &io) < 0 ||
>          vshCommandOptStringReq(ctl, cmd, "serial", &serial) < 0 ||
>          vshCommandOptStringReq(ctl, cmd, "wwn", &wwn) < 0 ||
>          vshCommandOptStringReq(ctl, cmd, "address", &straddr) < 0 ||
> @@ -579,7 +585,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
>      virBufferAddLit(&buf, ">\n");
>      virBufferAdjustIndent(&buf, 2);
> 
> -    if (driver || subdriver || iothread || cache) {
> +    if (driver || subdriver || iothread || cache || io) {
>          virBufferAddLit(&buf, "<driver");
> 
>          if (driver)
> @@ -590,6 +596,8 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
>              virBufferAsprintf(&buf, " iothread='%s'", iothread);
>          if (cache)
>              virBufferAsprintf(&buf, " cache='%s'", cache);
> +        if (io)
> +            virBufferAsprintf(&buf, " io='%s'", io);
> 
>          virBufferAddLit(&buf, "/>\n");
>      }
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index cd1f25f..9656411 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod


above here... there's the :

=item B<attach-disk> I<domain> I<source> I<target> [[[I<--live>]
[I<--config>]
...

You'll need to an "[I<--io io>]" within the list somewhere.

> @@ -2860,6 +2860,8 @@ I<mode> can specify the two specific mode
> I<readonly> or I<shareable>.
>  I<sourcetype> can indicate the type of source (block|file)
>  I<cache> can be one of "default", "none", "writethrough", "writeback",
>  "directsync" or "unsafe".
> +I<io> io is "threads", or "native" and selects between pthread based disk
> +I/O and native Linux AIO.
>  I<iothread> is the number within the range of domain IOThreads to which
>  this disk may be attached (QEMU only).
>  I<serial> is the serial of disk device. I<wwn> is the wwn of disk device.

Feel free to liberally borrow from the formatdomain description:

The optional io attribute controls specific policies on I/O; qemu guests
support "threads" and "native".

John