[libvirt] [PATCH 2/8] qemu: support attachment of net device with boot index

[John Ferlan]

On 01/05/2015 02:29 AM, Wang Rui wrote:
> When we attach an interface to a running VM with boot index, we get a
> successful result. But in fact the boot index won't take effect. QEMU
> supported to set device's boot index online recently(since QEMU 2.2.0).
> 
> After this patch, the boot index will take effect after
> virDomainAttachDevice(Flags) API returning success.
> 
> Signed-off-by: Wang Rui <moon.wangrui at huawei.com>
> Signed-off-by: Zhou Yimin <zhouyimin at huawei.com>
> ---
>  src/qemu/qemu_hotplug.c | 33 +++++++++++++++++++++++++++++++++
>  src/qemu/qemu_hotplug.h |  4 ++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 7f93b9b..919a061 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1087,6 +1087,14 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
>          /* link set to down */
>      }
>  
> +    if (net->info.bootIndex > 0) {
> +        if (qemuDomainChangeBootIndex(driver, vm, &net->info,
> +                                      net->info.bootIndex) < 0) {
> +            virDomainAuditNet(vm, NULL, net, "attach", false);

[1] See note below...

> +            goto try_remove;
> +        }
> +    }
> +
>      virDomainAuditNet(vm, NULL, net, "attach", true);
>  
>      ret = 0;
> @@ -1853,6 +1861,31 @@ int qemuDomainChangeNetLinkState(virQEMUDriverPtr driver,
>  }
>  
>  int
> +qemuDomainChangeBootIndex(virQEMUDriverPtr driver,
> +                          virDomainObjPtr vm,
> +                          virDomainDeviceInfoPtr devInfo,
> +                          int newBootIndex)
> +{
> +    int ret = -1;
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +
> +    if (!devInfo->alias) {
> +        virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +                       _("can't change boot index: device alias not found"));

s/can't/cannot   (I know you copied it from ChangeNetLinkState)

> +        return -1;
> +    }
> +
> +    VIR_DEBUG("Change dev: %s boot index from %d to %d", devInfo->alias,
> +              devInfo->bootIndex, newBootIndex);

Interesting that from patch 1 on you checked "!name" in the subsequent
SetBootIndex call and failed. Since you make that check above all the
more reason to go with the ATTRIBUTE_NONNULL as I suggested.

> +
> +    qemuDomainObjEnterMonitor(driver, vm);
> +    ret = qemuMonitorSetBootIndex(priv->mon, devInfo->alias, newBootIndex);
> +    qemuDomainObjExitMonitor(driver, vm);

[1] Due to commit id '5c703ca39' a check of the return is required.
However, it seems that this can follow other code from the calling
function and do an 'ignore_value();'.

If ExitMonitor returns < 0, then the vm is dead - that could cause
issues in the calling path which makes a virDomainNetAudit() call. Other
error path code from the caller will ignore status and audit, so I
suppose this could follow that, but it may not be as safe as expected.


> +
> +    return ret;
> +}
> +
> +int
>  qemuDomainChangeNet(virQEMUDriverPtr driver,
>                      virDomainObjPtr vm,
>                      virDomainPtr dom,
> diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
> index d13c532..3af0875 100644
> --- a/src/qemu/qemu_hotplug.h
> +++ b/src/qemu/qemu_hotplug.h
> @@ -57,6 +57,10 @@ int qemuDomainAttachHostDevice(virConnectPtr conn,
>                                 virDomainHostdevDefPtr hostdev);
>  int qemuDomainFindGraphicsIndex(virDomainDefPtr def,
>                                  virDomainGraphicsDefPtr dev);
> +int qemuDomainChangeBootIndex(virQEMUDriverPtr driver,
> +                              virDomainObjPtr vm,
> +                              virDomainDeviceInfoPtr devInfo,
> +                              int newBootIndex);

s/);/)
    ATTRIBUTE_NONNULL(3);

especially since you access it without checking for it...

John
>  int qemuDomainChangeGraphics(virQEMUDriverPtr driver,
>                               virDomainObjPtr vm,
>                               virDomainGraphicsDefPtr dev);
>