[libvirt] [PATCH] virsh: add --io when attaching disks to guests

Gordon Messmer posted 1 patch 6 years, 10 months ago
Failed in applying to current master (apply log)
tools/virsh-domain.c | 14 +++++++++++---
tools/virsh.pod      |  3 ++-
2 files changed, 13 insertions(+), 4 deletions(-)
[libvirt] [PATCH] virsh: add --io when attaching disks to guests
Posted by Gordon Messmer 6 years, 10 months ago
virt-install and virt-manager both default to explicitly setting
"io='native'" in the disk "driver" tag. virsh, however, does not and also
does not provide an option to specify that setting at all.  As a result,
disks use a different IO mechanism (the default, "threads") when attached
post-setup using virsh.  Adding this option allows users to keep disk
performance consistent for disks attached at install, and those attached
afterward.
---
 tools/virsh-domain.c | 14 +++++++++++---
 tools/virsh.pod      |  3 ++-
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 0d19d0e..5c42021 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 policy of disk device")
+    },
     {.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 727acf6..f46fb4a 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2838,7 +2838,7 @@ expected.
 =item B<attach-disk> I<domain> I<source> I<target> [[[I<--live>] [I<--config>]
 | [I<--current>]] | [I<--persistent>]] [I<--targetbus bus>] [I<--driver
 driver>] [I<--subdriver subdriver>] [I<--iothread iothread>]
-[I<--cache cache>] [I<--type type>]
+[I<--cache cache>] [I<--io io>] [I<--type type>]
 [I<--mode mode>] [I<--sourcetype sourcetype>] [I<--serial serial>] [I<--wwn
 wwn>] [I<--rawio>] [I<--address address>] [I<--multifunction>] [I<--print-xml>]
 
@@ -2865,6 +2865,7 @@ 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> controls specific policies on I/O; qemu guests support "threads" and "native".
 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.
-- 
2.9.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: add --io when attaching disks to guests
Posted by Gordon Messmer 6 years, 10 months ago
Please let me know if this patch requires additional revisions.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: add --io when attaching disks to guests
Posted by John Ferlan 6 years, 10 months ago

On 05/12/2017 05:27 PM, Gordon Messmer wrote:
> virt-install and virt-manager both default to explicitly setting
> "io='native'" in the disk "driver" tag. virsh, however, does not and also
> does not provide an option to specify that setting at all.  As a result,
> disks use a different IO mechanism (the default, "threads") when attached
> post-setup using virsh.  Adding this option allows users to keep disk
> performance consistent for disks attached at install, and those attached
> afterward.
> ---
>  tools/virsh-domain.c | 14 +++++++++++---
>  tools/virsh.pod      |  3 ++-
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 

Reviewed-by: John Ferlan <jferlan@redhat.com>

(and pushed w/ one minor change noted below)

Thanks and congrats on your first patch,

John

> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 0d19d0e..5c42021 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 policy of disk device")
> +    },
>      {.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 727acf6..f46fb4a 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -2838,7 +2838,7 @@ expected.
>  =item B<attach-disk> I<domain> I<source> I<target> [[[I<--live>] [I<--config>]
>  | [I<--current>]] | [I<--persistent>]] [I<--targetbus bus>] [I<--driver
>  driver>] [I<--subdriver subdriver>] [I<--iothread iothread>]
> -[I<--cache cache>] [I<--type type>]
> +[I<--cache cache>] [I<--io io>] [I<--type type>]
>  [I<--mode mode>] [I<--sourcetype sourcetype>] [I<--serial serial>] [I<--wwn
>  wwn>] [I<--rawio>] [I<--address address>] [I<--multifunction>] [I<--print-xml>]
>  
> @@ -2865,6 +2865,7 @@ 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> controls specific policies on I/O; qemu guests support "threads" and "native".

Altered the "qemu" to be "QEMU"

>  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.
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: add --io when attaching disks to guests
Posted by Gordon Messmer 6 years, 10 months ago
On 05/17/2017 10:51 AM, John Ferlan wrote:
> Thanks and congrats on your first patch,


Thanks for reviewing the patch and shepherding it through the process.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list