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@xxxxxxxxxx>
> Signed-off-by: Zhou Yimin <zhouyimin@xxxxxxxxxx>
> ---
> 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);
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list