[libvirt] [PATCH] virsh: support multifunction in attach-disk

From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> PCI <address...> can be specified by attach-disk but multifunction cannot be specified. add --multifunction support. --- tools/virsh.c | 7 ++++++- tools/virsh.pod | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index d58b827..346b440 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12661,6 +12661,7 @@ static const vshCmdOptDef opts_attach_disk[] = { {"serial", VSH_OT_STRING, 0, N_("serial of disk device")}, {"shareable", VSH_OT_BOOL, 0, N_("shareable between domains")}, {"address", VSH_OT_STRING, 0, N_("address of disk device")}, + {"multifunction", VSH_OT_BOOL, 0, N_("use multifunction pci under specified address")}, {NULL, 0, 0, NULL} }; @@ -12916,9 +12917,13 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (diskAddr.type == DISK_ADDR_TYPE_PCI) { virBufferAsprintf(&buf, " <address type='pci' domain='0x%04x'" - " bus ='0x%02x' slot='0x%02x' function='0x%0x' />\n", + " bus ='0x%02x' slot='0x%02x' function='0x%0x'", diskAddr.addr.pci.domain, diskAddr.addr.pci.bus, diskAddr.addr.pci.slot, diskAddr.addr.pci.function); + if (vshCommandOptBool(cmd, "multifunction")) + virBufferAsprintf(&buf, " multifunction='on' />\n"); + else + virBufferAsprintf(&buf, " />\n"); } else { vshError(ctl, "%s", _("expecting a pci:0000.00.00.00 address.")); goto cleanup; diff --git a/tools/virsh.pod b/tools/virsh.pod index fe92714..1a778f9 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1273,7 +1273,7 @@ the device does not use managed mode. =item B<attach-disk> I<domain-id> I<source> I<target> [I<--driver driver>] [I<--subdriver subdriver>] [I<--cache cache>] [I<--type type>] [I<--mode mode>] [I<--persistent>] [I<--sourcetype soucetype>] -[I<--serial serial>] [I<--shareable>] [I<--address address>] +[I<--serial serial>] [I<--shareable>] [I<--address address>] [I<--multifunction>] Attach a new disk device to the domain. I<source> and I<target> are paths for the files and devices. @@ -1291,6 +1291,7 @@ I<serial> is the serial of disk device. I<shareable> indicates the disk device is shareable between domains. I<address> is the address of disk device in the form of pci:domain.bus.slot.function, scsi:controller.bus.unit or ide:controller.bus.unit. +I<multifunction> indicates specified pci address is a multifunction pci device address. =item B<attach-interface> I<domain-id> I<type> I<source> [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>] -- 1.7.4.1

At 12/13/2011 02:12 PM, KAMEZAWA Hiroyuki Write:
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
PCI <address...> can be specified by attach-disk but multifunction cannot be specified. add --multifunction support.
IIRC, libvirt does not support hot plugging multifunction PCI device. Why do you need this feature? Thanks Wen Congyang
--- tools/virsh.c | 7 ++++++- tools/virsh.pod | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index d58b827..346b440 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12661,6 +12661,7 @@ static const vshCmdOptDef opts_attach_disk[] = { {"serial", VSH_OT_STRING, 0, N_("serial of disk device")}, {"shareable", VSH_OT_BOOL, 0, N_("shareable between domains")}, {"address", VSH_OT_STRING, 0, N_("address of disk device")}, + {"multifunction", VSH_OT_BOOL, 0, N_("use multifunction pci under specified address")}, {NULL, 0, 0, NULL} };
@@ -12916,9 +12917,13 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (diskAddr.type == DISK_ADDR_TYPE_PCI) { virBufferAsprintf(&buf, " <address type='pci' domain='0x%04x'" - " bus ='0x%02x' slot='0x%02x' function='0x%0x' />\n", + " bus ='0x%02x' slot='0x%02x' function='0x%0x'", diskAddr.addr.pci.domain, diskAddr.addr.pci.bus, diskAddr.addr.pci.slot, diskAddr.addr.pci.function); + if (vshCommandOptBool(cmd, "multifunction")) + virBufferAsprintf(&buf, " multifunction='on' />\n"); + else + virBufferAsprintf(&buf, " />\n"); } else { vshError(ctl, "%s", _("expecting a pci:0000.00.00.00 address.")); goto cleanup; diff --git a/tools/virsh.pod b/tools/virsh.pod index fe92714..1a778f9 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1273,7 +1273,7 @@ the device does not use managed mode. =item B<attach-disk> I<domain-id> I<source> I<target> [I<--driver driver>] [I<--subdriver subdriver>] [I<--cache cache>] [I<--type type>] [I<--mode mode>] [I<--persistent>] [I<--sourcetype soucetype>] -[I<--serial serial>] [I<--shareable>] [I<--address address>] +[I<--serial serial>] [I<--shareable>] [I<--address address>] [I<--multifunction>]
Attach a new disk device to the domain. I<source> and I<target> are paths for the files and devices. @@ -1291,6 +1291,7 @@ I<serial> is the serial of disk device. I<shareable> indicates the disk device is shareable between domains. I<address> is the address of disk device in the form of pci:domain.bus.slot.function, scsi:controller.bus.unit or ide:controller.bus.unit. +I<multifunction> indicates specified pci address is a multifunction pci device address.
=item B<attach-interface> I<domain-id> I<type> I<source> [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>]

On Tue, 13 Dec 2011 14:51:58 +0800 Wen Congyang <wency@cn.fujitsu.com> wrote:
At 12/13/2011 02:12 PM, KAMEZAWA Hiroyuki Write:
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
PCI <address...> can be specified by attach-disk but multifunction cannot be specified. add --multifunction support.
IIRC, libvirt does not support hot plugging multifunction PCI device. Why do you need this feature?
live-update is unnecessary. For updating _stopped_ domain's definition with virsh script. Thanks, -Kame

On 12/13/2011 12:14 AM, KAMEZAWA Hiroyuki wrote:
On Tue, 13 Dec 2011 14:51:58 +0800 Wen Congyang <wency@cn.fujitsu.com> wrote:
At 12/13/2011 02:12 PM, KAMEZAWA Hiroyuki Write:
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
PCI <address...> can be specified by attach-disk but multifunction cannot be specified. add --multifunction support.
IIRC, libvirt does not support hot plugging multifunction PCI device. Why do you need this feature?
Libvirt might not support it now, but there's no reason we can't add that support in the future if qemu is taught to be smart enough to handle it properly.
live-update is unnecessary. For updating _stopped_ domain's definition with virsh script.
Yep, a convincing enough reason to implement this virsh hook now. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 12/12/2011 11:12 PM, KAMEZAWA Hiroyuki wrote:
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
PCI <address...> can be specified by attach-disk but multifunction cannot be specified. add --multifunction support. --- tools/virsh.c | 7 ++++++- tools/virsh.pod | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-)
ACK and pushed.
diff --git a/tools/virsh.c b/tools/virsh.c index d58b827..346b440 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12661,6 +12661,7 @@ static const vshCmdOptDef opts_attach_disk[] = { {"serial", VSH_OT_STRING, 0, N_("serial of disk device")}, {"shareable", VSH_OT_BOOL, 0, N_("shareable between domains")}, {"address", VSH_OT_STRING, 0, N_("address of disk device")}, + {"multifunction", VSH_OT_BOOL, 0, N_("use multifunction pci under specified address")},
I wrapped this to fit 80 columns.
+ if (vshCommandOptBool(cmd, "multifunction")) + virBufferAsprintf(&buf, " multifunction='on' />\n"); + else + virBufferAsprintf(&buf, " />\n");
virBufferAsprintf for a string literal is heavy-weight; I swapped this to virBufferAddLit. diff --git i/tools/virsh.c w/tools/virsh.c index c2f8da3..f6571f7 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -12667,7 +12667,8 @@ static const vshCmdOptDef opts_attach_disk[] = { {"serial", VSH_OT_STRING, 0, N_("serial of disk device")}, {"shareable", VSH_OT_BOOL, 0, N_("shareable between domains")}, {"address", VSH_OT_STRING, 0, N_("address of disk device")}, - {"multifunction", VSH_OT_BOOL, 0, N_("use multifunction pci under specified address")}, + {"multifunction", VSH_OT_BOOL, 0, + N_("use multifunction pci under specified address")}, {NULL, 0, 0, NULL} }; @@ -12927,9 +12928,8 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) diskAddr.addr.pci.domain, diskAddr.addr.pci.bus, diskAddr.addr.pci.slot, diskAddr.addr.pci.function); if (vshCommandOptBool(cmd, "multifunction")) - virBufferAsprintf(&buf, " multifunction='on' />\n"); - else - virBufferAsprintf(&buf, " />\n"); + virBufferAddLit(&buf, " multifunction='on'"); + virBufferAddLit(&buf, "/>\n"); } else { vshError(ctl, "%s", _("expecting a pci:0000.00.00.00 address.")); goto cleanup; -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Tue, 13 Dec 2011 16:14:18 -0700 Eric Blake <eblake@redhat.com> wrote:
On 12/12/2011 11:12 PM, KAMEZAWA Hiroyuki wrote:
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
PCI <address...> can be specified by attach-disk but multifunction cannot be specified. add --multifunction support. --- tools/virsh.c | 7 ++++++- tools/virsh.pod | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-)
ACK and pushed.
Thank you.
diff --git a/tools/virsh.c b/tools/virsh.c index d58b827..346b440 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12661,6 +12661,7 @@ static const vshCmdOptDef opts_attach_disk[] = { {"serial", VSH_OT_STRING, 0, N_("serial of disk device")}, {"shareable", VSH_OT_BOOL, 0, N_("shareable between domains")}, {"address", VSH_OT_STRING, 0, N_("address of disk device")}, + {"multifunction", VSH_OT_BOOL, 0, N_("use multifunction pci under specified address")},
I wrapped this to fit 80 columns.
+ if (vshCommandOptBool(cmd, "multifunction")) + virBufferAsprintf(&buf, " multifunction='on' />\n"); + else + virBufferAsprintf(&buf, " />\n");
virBufferAsprintf for a string literal is heavy-weight; I swapped this to virBufferAddLit.
I'll write attach-interface patch carefully. -Kame
participants (3)
-
Eric Blake
-
KAMEZAWA Hiroyuki
-
Wen Congyang