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.