[libvirt] [PATCH 7/8] qemu: support updating interface with boot index

[John Ferlan]

On 01/05/2015 02:29 AM, Wang Rui wrote:
> QEMU supported to set device's boot index online recently(since QEMU 2.2.0).
> This patch implements the interface's boot index update lively.
> 
> If PCI address is not specified in the new xml, we can also update boot
> index. If boot order is not specified in the new xml, we take it as canceling
> boot index. So pass "value":-1 to qmp command("qom-set") to cancel boot index.
> 
> Signed-off-by: Wang Rui <moon.wangrui at huawei.com>
> Signed-off-by: Zhou Yimin <zhouyimin at huawei.com>
> ---
>  src/qemu/qemu_hotplug.c | 35 ++++++++++++++++++++++++++++++-----
>  1 file changed, 30 insertions(+), 5 deletions(-)
> 

So now we're adding the same functionality to a network interface?
Maybe a better description above would help me understand what you're
trying to accomplish.

Perhaps let's get the disk devices to be all set before taking this path
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 5eacfce..4c86370 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1814,6 +1814,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
>      bool needLinkStateChange = false;
>      bool needReplaceDevDef = false;
>      bool needBandwidthSet = false;
> +    bool needBootIndexChange = false;
>      int ret = -1;
>  
>      if (!devslot || !(olddev = *devslot)) {
> @@ -1909,6 +1910,19 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
>          goto cleanup;
>      }
>  
> +    /* If(and olny if) PCI adderss is not specified in the new xml, newdev->info.type

s/If(and olny if)/If (and only if) a
s/adderss/address


Going back a few patches for attach & update and my concern over whether
the XML has an address already or not - this shows why I was concerned.
 If you don't yet have an address you could possibly match something
that you weren't expecting to or not match something...

I didn't look too much deeper here.

BTW: If you're adding network, you'll need to document that.

John
> +     * here is NONE. We can copy the old device's PCI address to the new one. We can't
> +     * copy olddev->info to newdev->info, because other members in newdev->info(such
> +     * as bootIndex) should not be overridden.
> +     */
> +    if (newdev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
> +        newdev->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> +        newdev->info.addr.pci.domain = olddev->info.addr.pci.domain;
> +        newdev->info.addr.pci.bus = olddev->info.addr.pci.bus;
> +        newdev->info.addr.pci.slot = olddev->info.addr.pci.slot;
> +        newdev->info.addr.pci.function = olddev->info.addr.pci.function;
> +    }
> +
>      /* info: if newdev->info is empty, fill it in from olddev,
>       * otherwise verify that it matches - nothing is allowed to
>       * change. (There is no helper function to do this, so
> @@ -1945,11 +1959,6 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
>                         _("cannot modify network rom file"));
>          goto cleanup;
>      }
> -    if (olddev->info.bootIndex != newdev->info.bootIndex) {
> -        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> -                       _("cannot modify network device boot index setting"));
> -        goto cleanup;
> -    }
>      /* (end of device info checks) */
>  
>      if (STRNEQ_NULLABLE(olddev->filter, newdev->filter) ||
> @@ -2101,6 +2110,9 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
>                                   virDomainNetGetActualBandwidth(newdev)))
>          needBandwidthSet = true;
>  
> +    if (olddev->info.bootIndex != newdev->info.bootIndex)
> +        needBootIndexChange = true;
> +
>      /* FINALLY - actually perform the required actions */
>  
>      if (needReconnect) {
> @@ -2141,6 +2153,19 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
>          goto cleanup;
>      }
>  
> +    if (needBootIndexChange) {
> +        /* If boot index is to be changed to 0, we can pass "value":-1 to
> +           qmp command("qom-set") to cancel boot index. */
> +        if (qemuDomainChangeBootIndex(driver, vm, &olddev->info,
> +                                      newdev->info.bootIndex ?
> +                                      newdev->info.bootIndex : -1) < 0)
> +            goto cleanup;
> +        /* we successfully switched to the new boot index, and we've
> +         * determined that the rest of newdev is equivalent to olddev,
> +         * so move newdev into place */
> +        needReplaceDevDef = true;
> +    }
> +
>      if (needReplaceDevDef) {
>          /* the changes above warrant replacing olddev with newdev in
>           * the domain's nets list.
>