1:29 a.m.
QEMU supported to set device's boot index online recently(since QEMU 2.2.0).
http://lists.gnu.org/archive/html/qemu-devel/2014-10/msg00879.html
The QMP "qom-set" is available for setting boot index. qom-set's example is
as follows:
{
.name = "qom-set",
.args_type = "path:s,property:s,value:q",
.mhandler.cmd_new = qmp_qom_set,
}
(qemu) qom-set nic1 bootindex 3
(qemu) qom-set nic1 bootindex -1
(qemu) qom-set scsi0-0-0-0 bootindex 2
Setting boot index to -1 means cancel it.
We can attach/update a device and set it's boot order when VM is running. The boot
order will take effect immediately(after guest rebooting). For example, we can use
'virsh update-device' to change an interface's boot order. The xml is
specified as
below.
<interface type='bridge'>
<mac address='fa:16:3e:2e:d4:3d'/>
<source bridge='br0'/>
<target dev='tap4f2bce78-5b'/>
<model type='virtio'/>
<driver queues='4'/>
<boot order='2'/>
<alias name='net0'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
</interface>
If the 'boot order' is not specified in the xml, that means canceling its boot
order.
Wang Rui (8):
qemu: add a new qemuMonitorSetBootIndex() method to set device's
bootorder
qemu: support attachment of net device with boot index
conf: check boot order which is used by itself
qemu: a code movement
qemu: support attachment of disk device with boot index
conf: add compatiblity check for boot index when updating device
qemu: support updating interface with boot index
qemu: support updating disk with boot index
src/conf/device_conf.c | 13 ++
src/conf/device_conf.h | 12 ++
src/conf/domain_conf.c | 74 ++++++----
src/conf/domain_conf.h | 9 --
src/qemu/qemu_driver.c | 64 +++++++--
src/qemu/qemu_hotplug.c | 329 +++++++++++++++++++++++++++++++------------
src/qemu/qemu_hotplug.h | 4 +
src/qemu/qemu_monitor.c | 25 ++++
src/qemu/qemu_monitor.h | 4 +
src/qemu/qemu_monitor_json.c | 19 +++
src/qemu/qemu_monitor_json.h | 5 +
11 files changed, 424 insertions(+), 134 deletions(-)
--
1.7.12.4
1:29 a.m.
New subject: [libvirt] [PATCH 1/8] qemu: add a new qemuMonitorSetBootIndex() method to set device's bootorder
The new qemuMonitorSetBootIndex() method can set device' boot order online
using 'qom-set' JSON monitor command. HMP is not supported. And it is used
for QEMU >= 2.2.0 . The QMP command is like "qom-set net1 bootindex 2".
Signed-off-by: Wang Rui <moon.wangrui(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com>
---
src/qemu/qemu_monitor.c | 25 +++++++++++++++++++++++++
src/qemu/qemu_monitor.h | 4 ++++
src/qemu/qemu_monitor_json.c | 19 +++++++++++++++++++
src/qemu/qemu_monitor_json.h | 5 +++++
4 files changed, 53 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 100bbd0..907834f 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4292,3 +4292,28 @@ void qemuMonitorIOThreadsInfoFree(qemuMonitorIOThreadsInfoPtr
iothread)
VIR_FREE(iothread->name);
VIR_FREE(iothread);
}
+
+int
+qemuMonitorSetBootIndex(qemuMonitorPtr mon,
+ const char *name,
+ int bootIndex)
+{
+ int ret;
+ VIR_DEBUG("mon=%p, name=%p:%s, bootIndex=%d", mon, name, name, bootIndex);
+
+ if (!mon || !name) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor || name must not be NULL"));
+ return -1;
+ }
+
+ if (!mon->json) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("JSON monitor is required"));
+ return -1;
+ }
+
+ ret = qemuMonitorJSONSetBootIndex(mon, name, bootIndex);
+
+ return ret;
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index edab66f..8e5d86e 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -888,6 +888,10 @@ int qemuMonitorGetIOThreads(qemuMonitorPtr mon,
void qemuMonitorIOThreadsInfoFree(qemuMonitorIOThreadsInfoPtr iothread);
+int qemuMonitorSetBootIndex(qemuMonitorPtr mon,
+ const char *name,
+ int bootIndex);
+
/**
* When running two dd process and using <> redirection, we need a
* shell that will not truncate files. These two strings serve that
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index e567aa7..df52101 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6500,3 +6500,22 @@ qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon,
virJSONValueFree(reply);
return ret;
}
+
+int
+qemuMonitorJSONSetBootIndex(qemuMonitorPtr mon,
+ const char *name,
+ int bootIndex)
+{
+ qemuMonitorJSONObjectProperty prop;
+
+ memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
+ prop.val.iv = bootIndex;
+
+ if (qemuMonitorJSONSetObjectProperty(mon, name,
+ "bootindex",
+ &prop) < 0) {
+ return -1;
+ }
+ return 0;
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 222f11e..6355ddf 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -472,4 +472,9 @@ int qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon);
int qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon,
qemuMonitorIOThreadsInfoPtr **iothreads)
ATTRIBUTE_NONNULL(2);
+
+int qemuMonitorJSONSetBootIndex(qemuMonitorPtr mon,
+ const char *name,
+ int bootIndex);
+
#endif /* QEMU_MONITOR_JSON_H */
--
1.7.12.4
7:38 p.m.
New subject: [libvirt] [PATCH 1/8] qemu: add a new qemuMonitorSetBootIndex() method to set device's bootorder
On 01/05/2015 02:29 AM, Wang Rui wrote:
The new qemuMonitorSetBootIndex() method can set device' boot
order online
using 'qom-set' JSON monitor command. HMP is not supported. And it is used
for QEMU >= 2.2.0 . The QMP command is like "qom-set net1 bootindex 2".
Signed-off-by: Wang Rui <moon.wangrui(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com>
---
src/qemu/qemu_monitor.c | 25 +++++++++++++++++++++++++
src/qemu/qemu_monitor.h | 4 ++++
src/qemu/qemu_monitor_json.c | 19 +++++++++++++++++++
src/qemu/qemu_monitor_json.h | 5 +++++
4 files changed, 53 insertions(+)
Looks like this was "missed/lost/forgotten" from the holidays... You'll
need to update with top of tree...
What happens if qemu < 2.2? Is there a way to test? Callers will end
up failing perhaps when you don't want them to...
After reading later patches - is there a similar qom-get for this field?
That would simplify a few nasty error paths later..
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 100bbd0..907834f 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4292,3 +4292,28 @@ void qemuMonitorIOThreadsInfoFree(qemuMonitorIOThreadsInfoPtr
iothread)
VIR_FREE(iothread->name);
VIR_FREE(iothread);
}
+
+int
+qemuMonitorSetBootIndex(qemuMonitorPtr mon,
+ const char *name,
+ int bootIndex)
+{
+ int ret;
+ VIR_DEBUG("mon=%p, name=%p:%s, bootIndex=%d", mon, name, name,
bootIndex);
+
+ if (!mon || !name) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor || name must not be NULL"));
+ return -1;
+ }
See [1]
+
+ if (!mon->json) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("JSON monitor is required"));
+ return -1;
+ }
+
+ ret = qemuMonitorJSONSetBootIndex(mon, name, bootIndex);
+
+ return ret;
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index edab66f..8e5d86e 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -888,6 +888,10 @@ int qemuMonitorGetIOThreads(qemuMonitorPtr mon,
void qemuMonitorIOThreadsInfoFree(qemuMonitorIOThreadsInfoPtr iothread);
+int qemuMonitorSetBootIndex(qemuMonitorPtr mon,
+ const char *name,
+ int bootIndex);
[1]
If you change this slightly to be
int bootIndex)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
Then you avoid the !mon || !name checks above.... also the %p:%s becomes
unnecessary - that is either %p or %s.
+
/**
* When running two dd process and using <> redirection, we need a
* shell that will not truncate files. These two strings serve that
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index e567aa7..df52101 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6500,3 +6500,22 @@ qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon,
virJSONValueFree(reply);
return ret;
}
+
+int
+qemuMonitorJSONSetBootIndex(qemuMonitorPtr mon,
+ const char *name,
+ int bootIndex)
+{
+ qemuMonitorJSONObjectProperty prop;
+
+ memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
+ prop.val.iv = bootIndex;
+
+ if (qemuMonitorJSONSetObjectProperty(mon, name,
+ "bootindex",
+ &prop) < 0) {
+ return -1;
+ }
+ return 0;
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 222f11e..6355ddf 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -472,4 +472,9 @@ int qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon);
int qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon,
qemuMonitorIOThreadsInfoPtr **iothreads)
ATTRIBUTE_NONNULL(2);
+
+int qemuMonitorJSONSetBootIndex(qemuMonitorPtr mon,
+ const char *name,
+ int bootIndex);
+
#endif /* QEMU_MONITOR_JSON_H */
1:29 a.m.
New subject: [libvirt] [PATCH 2/8] qemu: support attachment of net device with boot index
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(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)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);
+ 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"));
+ return -1;
+ }
+
+ VIR_DEBUG("Change dev: %s boot index from %d to %d", devInfo->alias,
+ devInfo->bootIndex, newBootIndex);
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ ret = qemuMonitorSetBootIndex(priv->mon, devInfo->alias, newBootIndex);
+ qemuDomainObjExitMonitor(driver, vm);
+
+ 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);
int qemuDomainChangeGraphics(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainGraphicsDefPtr dev);
--
1.7.12.4
7:39 p.m.
New subject: [libvirt] [PATCH 2/8] qemu: support attachment of net device with boot index
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(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)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);
1:29 a.m.
New subject: [libvirt] [PATCH 3/8] conf: check boot order which is used by itself
We can update device and attach device(cdrom) with the boot order unchanged.
But the boot order check method will report an error:
"boot order %d is already used by another device"
In fact the boot order is used by itself, we just want to keep it as before.
This patch improves boot order check to distinguish the owner.
Signed-off-by: Wang Rui <moon.wangrui(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com>
---
src/conf/device_conf.c | 13 +++++++++++++
src/conf/device_conf.h | 12 ++++++++++++
src/conf/domain_conf.c | 31 +++++++++++++++++++++++++++----
src/conf/domain_conf.h | 9 ---------
4 files changed, 52 insertions(+), 13 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index 5ffe159..907a7b8 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -144,6 +144,19 @@ virDevicePCIAddressEqual(virDevicePCIAddress *addr1,
return false;
}
+bool
+virDeviceDriveAddressEqual(virDomainDeviceDriveAddressPtr addr1,
+ virDomainDeviceDriveAddressPtr addr2)
+{
+ if (addr1->controller == addr2->controller &&
+ addr1->bus == addr2->bus &&
+ addr1->target == addr2->target &&
+ addr1->unit == addr2->unit) {
+ return true;
+ }
+ return false;
+}
+
int
virInterfaceLinkParseXML(xmlNodePtr node,
virInterfaceLinkPtr lnk)
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index f067a35..afcf0ee 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -55,6 +55,15 @@ struct _virDevicePCIAddress {
int multi; /* enum virDomainDeviceAddressPCIMulti */
};
+typedef struct _virDomainDeviceDriveAddress virDomainDeviceDriveAddress;
+typedef virDomainDeviceDriveAddress *virDomainDeviceDriveAddressPtr;
+struct _virDomainDeviceDriveAddress {
+ unsigned int controller;
+ unsigned int bus;
+ unsigned int target;
+ unsigned int unit;
+};
+
typedef struct _virInterfaceLink virInterfaceLink;
typedef virInterfaceLink *virInterfaceLinkPtr;
struct _virInterfaceLink {
@@ -74,6 +83,9 @@ int virDevicePCIAddressFormat(virBufferPtr buf,
bool virDevicePCIAddressEqual(virDevicePCIAddress *addr1,
virDevicePCIAddress *addr2);
+bool virDeviceDriveAddressEqual(virDomainDeviceDriveAddressPtr addr1,
+ virDomainDeviceDriveAddressPtr addr2);
+
int virInterfaceLinkParseXML(xmlNodePtr node,
virInterfaceLinkPtr lnk);
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index aafc05e..d2c4a0a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19953,12 +19953,35 @@ virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def
ATTRIBUTE_UNUSED,
virDomainDeviceInfoPtr newinfo = opaque;
if (info->bootIndex == newinfo->bootIndex) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("boot order %d is already used by another device"),
- newinfo->bootIndex);
- return -1;
+ /* check if the same boot order is used by another device or itself */
+ if (info->type != newinfo->type) {
+ goto error;
+ } else {
+ switch ((virDomainDeviceAddressType) newinfo->type) {
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
+ if (virDevicePCIAddressEqual(&info->addr.pci,
+ &newinfo->addr.pci))
+ return 0;
+ else
+ goto error;
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
+ if (virDeviceDriveAddressEqual(&info->addr.drive,
+ &newinfo->addr.drive))
+ return 0;
+ else
+ goto error;
+ default:
+ goto error;
+ }
+ }
}
return 0;
+
+ error:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("boot order %d is already used by another device"),
+ newinfo->bootIndex);
+ return -1;
}
int
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 57297cd..3bddc80 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -236,15 +236,6 @@ typedef enum {
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST
} virDomainDeviceAddressType;
-typedef struct _virDomainDeviceDriveAddress virDomainDeviceDriveAddress;
-typedef virDomainDeviceDriveAddress *virDomainDeviceDriveAddressPtr;
-struct _virDomainDeviceDriveAddress {
- unsigned int controller;
- unsigned int bus;
- unsigned int target;
- unsigned int unit;
-};
-
typedef struct _virDomainDeviceVirtioSerialAddress virDomainDeviceVirtioSerialAddress;
typedef virDomainDeviceVirtioSerialAddress *virDomainDeviceVirtioSerialAddressPtr;
struct _virDomainDeviceVirtioSerialAddress {
--
1.7.12.4
7:39 p.m.
New subject: [libvirt] [PATCH 3/8] conf: check boot order which is used by itself
On 01/05/2015 02:29 AM, Wang Rui wrote:
We can update device and attach device(cdrom) with the boot order
unchanged.
But the boot order check method will report an error:
"boot order %d is already used by another device"
In fact the boot order is used by itself, we just want to keep it as before.
This patch improves boot order check to distinguish the owner.
Signed-off-by: Wang Rui <moon.wangrui(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com>
---
src/conf/device_conf.c | 13 +++++++++++++
src/conf/device_conf.h | 12 ++++++++++++
src/conf/domain_conf.c | 31 +++++++++++++++++++++++++++----
src/conf/domain_conf.h | 9 ---------
4 files changed, 52 insertions(+), 13 deletions(-)
Probably could have split this in two in order to do code motion fi...
3a. -> Move virDomainDeviceDriveAddress from domain_conf.h to
device_conf.h which just seems to be separate from the... op
3b. -> Rest of this patch and you will also need to modify
src/libvirt_private.syms in order to add virDeviceDriveAddressEqual;
"properly" since it's global.
I also think that there's some synergies with patch 6/8 especially with
the matching logic
Perhaps an update to the "boot order" text in the <hostdev> section of
formatdomain.html.in would be beneficial to describe these additional
checks.
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index 5ffe159..907a7b8 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -144,6 +144,19 @@ virDevicePCIAddressEqual(virDevicePCIAddress *addr1,
return false;
}
+bool
+virDeviceDriveAddressEqual(virDomainDeviceDriveAddressPtr addr1,
+ virDomainDeviceDriveAddressPtr addr2)
+{
+ if (addr1->controller == addr2->controller &&
+ addr1->bus == addr2->bus &&
+ addr1->target == addr2->target &&
+ addr1->unit == addr2->unit) {
+ return true;
+ }
+ return false;
+}
+
int
virInterfaceLinkParseXML(xmlNodePtr node,
virInterfaceLinkPtr lnk)
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index f067a35..afcf0ee 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -55,6 +55,15 @@ struct _virDevicePCIAddress {
int multi; /* enum virDomainDeviceAddressPCIMulti */
};
+typedef struct _virDomainDeviceDriveAddress virDomainDeviceDriveAddress;
+typedef virDomainDeviceDriveAddress *virDomainDeviceDriveAddressPtr;
+struct _virDomainDeviceDriveAddress {
+ unsigned int controller;
+ unsigned int bus;
+ unsigned int target;
+ unsigned int unit;
+};
+
typedef struct _virInterfaceLink virInterfaceLink;
typedef virInterfaceLink *virInterfaceLinkPtr;
struct _virInterfaceLink {
@@ -74,6 +83,9 @@ int virDevicePCIAddressFormat(virBufferPtr buf,
bool virDevicePCIAddressEqual(virDevicePCIAddress *addr1,
virDevicePCIAddress *addr2);
+bool virDeviceDriveAddressEqual(virDomainDeviceDriveAddressPtr addr1,
+ virDomainDeviceDriveAddressPtr addr2);
+
int virInterfaceLinkParseXML(xmlNodePtr node,
virInterfaceLinkPtr lnk);
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index aafc05e..d2c4a0a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19953,12 +19953,35 @@ virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def
ATTRIBUTE_UNUSED,
virDomainDeviceInfoPtr newinfo = opaque;
if (info->bootIndex == newinfo->bootIndex) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("boot order %d is already used by another device"),
- newinfo->bootIndex);
- return -1;
+ /* check if the same boot order is used by another device or itself */
+ if (info->type != newinfo->type) {
+ goto error;
+ } else {
+ switch ((virDomainDeviceAddressType) newinfo->type) {
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
+ if (virDevicePCIAddressEqual(&info->addr.pci,
+ &newinfo->addr.pci))
+ return 0;
+ else
+ goto error;
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
+ if (virDeviceDriveAddressEqual(&info->addr.drive,
+ &newinfo->addr.drive))
+ return 0;
+ else
+ goto error;
+ default:
+ goto error;
This is difficult to read with all the if-then-else and goto's.
How about the following:
if (info->bootIndex == newinfo->bootIndex) {
/* check if the same boot order is used by another device or
itself */
if (info->type != newinfo->type)
goto error;
if ((newinfo->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
virDevicePCIAddressEqual(&info->addr.pci,
&newinfo->addr.pci)) ||
(newinfo->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE &&
virDeviceDriveAddressEqual(&info->addr.drive,
&newinfo->addr.drive)))
return 0;
goto error;
}
return 0;
error:
...
Even with this change - I'm concerned about the cases where 'type'
doesn't match and is not PCI or DRIVE (e.g., the two new error paths).
Trying to wrap my mind about the timing of the check and when the 'type'
and address value are filled in w/r/t if they're not present in the XML
when the guest starts, but I think this is only called in an attach,
update, or detach path so it should be ok... Although I see
virDomainDefCompatibleDevice is called in an LXC path, so while I assume
you've made sure the qemu path works right, what about the lxc path as
it relates to the device info type/address checks?
Since patch 6 changes the Iterate from just 'attach' to 'attach' or
'update', it seems you're trying to "cover" the update case, but
may
have caused unforeseen issues with the attach case.
I think my 3b and 6 should be combined to make it easier to consider the
options.
In particular, it seems new restrictions are in place that bootIndex can
only work for PCI and DRIVE virDomainDeviceInfo address types. I think
the USB type is throwing me off a bit.
Just trying to make sure some already existing option isn't being
disallowed with the new changes. Adding in specific device address
checks based on types just raises that concern.
John
+ }
+ }
}
return 0;
+
+ error:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("boot order %d is already used by another device"),
+ newinfo->bootIndex);
+ return -1;
}
int
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 57297cd..3bddc80 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -236,15 +236,6 @@ typedef enum {
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST
} virDomainDeviceAddressType;
-typedef struct _virDomainDeviceDriveAddress virDomainDeviceDriveAddress;
-typedef virDomainDeviceDriveAddress *virDomainDeviceDriveAddressPtr;
-struct _virDomainDeviceDriveAddress {
- unsigned int controller;
- unsigned int bus;
- unsigned int target;
- unsigned int unit;
-};
-
typedef struct _virDomainDeviceVirtioSerialAddress virDomainDeviceVirtioSerialAddress;
typedef virDomainDeviceVirtioSerialAddress *virDomainDeviceVirtioSerialAddressPtr;
struct _virDomainDeviceVirtioSerialAddress {
1:29 a.m.
New subject: [libvirt] [PATCH 4/8] qemu: a code movement
Move implementation of qemuDomainAttachDeviceDiskLive after
qemuDomainDetachDiskDevice for later usage.
Signed-off-by: Wang Rui <moon.wangrui(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com>
---
src/qemu/qemu_hotplug.c | 173 ++++++++++++++++++++++++------------------------
1 file changed, 86 insertions(+), 87 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 919a061..2f84949 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -729,93 +729,6 @@ qemuDomainAttachUSBMassstorageDevice(virConnectPtr conn,
}
-int
-qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
- virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- virDomainDeviceDefPtr dev)
-{
- virDomainDiskDefPtr disk = dev->data.disk;
- virDomainDiskDefPtr orig_disk = NULL;
- int ret = -1;
- const char *driverName = virDomainDiskGetDriver(disk);
- const char *src = virDomainDiskGetSource(disk);
-
- if (driverName && !STREQ(driverName, "qemu")) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported driver name '%s' for disk
'%s'"),
- driverName, src);
- goto end;
- }
-
- if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
- goto end;
-
- if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0)
- goto end;
-
- if (qemuSetUnprivSGIO(dev) < 0)
- goto end;
-
- if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
- goto end;
-
- switch (disk->device) {
- case VIR_DOMAIN_DISK_DEVICE_CDROM:
- case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
- if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def,
- disk->bus, disk->dst))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("No device with bus '%s' and target
'%s'. "
- "cdrom and floppy device hotplug isn't supported
"
- "by libvirt"),
- virDomainDiskBusTypeToString(disk->bus),
- disk->dst);
- goto end;
- }
-
- if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk,
- disk->src, false) < 0)
- goto end;
-
- disk->src = NULL;
- ret = 0;
- break;
-
- case VIR_DOMAIN_DISK_DEVICE_DISK:
- case VIR_DOMAIN_DISK_DEVICE_LUN:
- if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
- if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("disk device='lun' is not supported for usb
bus"));
- break;
- }
- ret = qemuDomainAttachUSBMassstorageDevice(conn, driver, vm,
- disk);
- } else if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
- ret = qemuDomainAttachVirtioDiskDevice(conn, driver, vm, disk);
- } else if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
- ret = qemuDomainAttachSCSIDisk(conn, driver, vm, disk);
- } else {
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
- _("disk bus '%s' cannot be hotplugged."),
- virDomainDiskBusTypeToString(disk->bus));
- }
- break;
- default:
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
- _("disk device type '%s' cannot be
hotplugged"),
- virDomainDiskDeviceTypeToString(disk->device));
- break;
- }
-
- end:
- if (ret != 0)
- ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name));
- return ret;
-}
-
-
/* XXX conn required for network -> bridge resolution */
int qemuDomainAttachNetDevice(virConnectPtr conn,
virQEMUDriverPtr driver,
@@ -3078,6 +2991,92 @@ qemuDomainDetachDiskDevice(virQEMUDriverPtr driver,
return ret;
}
+int
+qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
+ virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
+{
+ virDomainDiskDefPtr disk = dev->data.disk;
+ virDomainDiskDefPtr orig_disk = NULL;
+ int ret = -1;
+ const char *driverName = virDomainDiskGetDriver(disk);
+ const char *src = virDomainDiskGetSource(disk);
+
+ if (driverName && !STREQ(driverName, "qemu")) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported driver name '%s' for disk
'%s'"),
+ driverName, src);
+ goto end;
+ }
+
+ if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
+ goto end;
+
+ if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0)
+ goto end;
+
+ if (qemuSetUnprivSGIO(dev) < 0)
+ goto end;
+
+ if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
+ goto end;
+
+ switch (disk->device) {
+ case VIR_DOMAIN_DISK_DEVICE_CDROM:
+ case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
+ if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def,
+ disk->bus, disk->dst))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("No device with bus '%s' and target
'%s'. "
+ "cdrom and floppy device hotplug isn't supported
"
+ "by libvirt"),
+ virDomainDiskBusTypeToString(disk->bus),
+ disk->dst);
+ goto end;
+ }
+
+ if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk,
+ disk->src, false) < 0)
+ goto end;
+
+ disk->src = NULL;
+ ret = 0;
+ break;
+
+ case VIR_DOMAIN_DISK_DEVICE_DISK:
+ case VIR_DOMAIN_DISK_DEVICE_LUN:
+ if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
+ if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("disk device='lun' is not supported for usb
bus"));
+ break;
+ }
+ ret = qemuDomainAttachUSBMassstorageDevice(conn, driver, vm,
+ disk);
+ } else if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
+ ret = qemuDomainAttachVirtioDiskDevice(conn, driver, vm, disk);
+ } else if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
+ ret = qemuDomainAttachSCSIDisk(conn, driver, vm, disk);
+ } else {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+ _("disk bus '%s' cannot be hotplugged."),
+ virDomainDiskBusTypeToString(disk->bus));
+ }
+ break;
+ default:
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+ _("disk device type '%s' cannot be
hotplugged"),
+ virDomainDiskDeviceTypeToString(disk->device));
+ break;
+ }
+
+ end:
+ if (ret != 0)
+ ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name));
+ return ret;
+}
+
static int
qemuFindDisk(virDomainDefPtr def, const char *dst)
{
--
1.7.12.4
7:39 p.m.
New subject: [libvirt] [PATCH 4/8] qemu: a code movement
On 01/05/2015 02:29 AM, Wang Rui wrote:
Move implementation of qemuDomainAttachDeviceDiskLive after
qemuDomainDetachDiskDevice for later usage.
Signed-off-by: Wang Rui <moon.wangrui(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com>
---
src/qemu/qemu_hotplug.c | 173 ++++++++++++++++++++++++------------------------
1 file changed, 86 insertions(+), 87 deletions(-)
Code motion - ACK.
John
1:29 a.m.
New subject: [libvirt] [PATCH 5/8] qemu: support attachment of disk device with boot index
When we attach a disk to a running VM with boot index, we can 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. If new disk is
attached successfully but boot index is set failed, we'll remove the
new disk to restore.
Signed-off-by: Wang Rui <moon.wangrui(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com>
---
src/qemu/qemu_hotplug.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 88 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 2f84949..5eacfce 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2999,7 +2999,12 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
{
virDomainDiskDefPtr disk = dev->data.disk;
virDomainDiskDefPtr orig_disk = NULL;
+ virDomainDeviceDefPtr new_dev_copy = NULL;
+ virDomainDeviceDefPtr old_dev_copy = NULL;
+ virDomainDiskDefPtr tmp = NULL;
+ virCapsPtr caps = NULL;
int ret = -1;
+ int removed = 0;
const char *driverName = virDomainDiskGetDriver(disk);
const char *src = virDomainDiskGetSource(disk);
@@ -3022,6 +3027,13 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
goto end;
+ if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+ goto end;
+
+ if (!(new_dev_copy = virDomainDeviceDefCopy(dev, vm->def,
+ caps, driver->xmlopt)))
+ goto end;
+
switch (disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
@@ -3036,12 +3048,33 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
goto end;
}
+ tmp = dev->data.disk;
+ dev->data.disk = orig_disk;
+ /* save a copy of old device to restore */
+ if (!(old_dev_copy = virDomainDeviceDefCopy(dev, vm->def,
+ caps, driver->xmlopt))) {
+ dev->data.disk = tmp;
+ goto end;
+ }
+ dev->data.disk = tmp;
+
if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk,
disk->src, false) < 0)
goto end;
disk->src = NULL;
ret = 0;
+
+ tmp = new_dev_copy->data.disk;
+ if (orig_disk->info.bootIndex != tmp->info.bootIndex) {
+ /* 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, &orig_disk->info,
+ tmp->info.bootIndex ?
+ tmp->info.bootIndex : -1) < 0)
+ goto try_remove;
+ orig_disk->info.bootIndex = tmp->info.bootIndex;
+ }
break;
case VIR_DOMAIN_DISK_DEVICE_DISK:
@@ -3063,6 +3096,15 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
_("disk bus '%s' cannot be hotplugged."),
virDomainDiskBusTypeToString(disk->bus));
}
+
+ tmp = new_dev_copy->data.disk;
+ if (!ret && tmp->info.bootIndex > 0) {
+ if (qemuDomainChangeBootIndex(driver, vm, &disk->info,
+ tmp->info.bootIndex) < 0)
+ goto try_remove;
+ disk->info.bootIndex = tmp->info.bootIndex;
+ }
+
break;
default:
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
@@ -3074,7 +3116,53 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
end:
if (ret != 0)
ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name));
+ virObjectUnref(caps);
+ virDomainDeviceDefFree(new_dev_copy);
+ virDomainDeviceDefFree(old_dev_copy);
+ if (removed) {
+ dev->data.disk = NULL;
+ ret = -1;
+ }
return ret;
+
+ try_remove:
+ if (!virDomainObjIsActive(vm)) {
+ removed = 1;
+ goto end;
+ }
+
+ switch (new_dev_copy->data.disk->device) {
+ case VIR_DOMAIN_DISK_DEVICE_CDROM:
+ case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
+ if (qemuAddSharedDevice(driver, old_dev_copy, vm->def->name) < 0)
+ break;
+
+ tmp = old_dev_copy->data.disk;
+ if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk, tmp->src,
false) < 0) {
+ VIR_WARN("Unable to recover original media with target '%s' and
path '%s'",
+ tmp->dst, tmp->src->path);
+ ignore_value(qemuRemoveSharedDevice(driver, old_dev_copy,
vm->def->name));
+ } else {
+ tmp->src = NULL;
+ }
+ break;
+ case VIR_DOMAIN_DISK_DEVICE_DISK:
+ case VIR_DOMAIN_DISK_DEVICE_LUN:
+ if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
+ if (qemuDomainDetachVirtioDiskDevice(driver, vm, disk) != 0)
+ VIR_WARN("Unable to detach new disk with bus '%s' and target
'%s'",
+ virDomainDiskBusTypeToString(disk->bus), disk->dst);
+ }
+ else if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI ||
+ disk->bus == VIR_DOMAIN_DISK_BUS_USB)
+ if (qemuDomainDetachDiskDevice(driver, vm, disk) != 0)
+ VIR_WARN("Unable to detach new disk with bus '%s' and target
'%s'",
+ virDomainDiskBusTypeToString(disk->bus), disk->dst);
+ ret = -1;
+ break;
+ }
+ removed = 1;
+ goto end;
}
static int
--
1.7.12.4
7:41 p.m.
New subject: [libvirt] [PATCH 5/8] qemu: support attachment of disk device with boot index
On 01/05/2015 02:29 AM, Wang Rui wrote:
When we attach a disk to a running VM with boot index, we can 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. If new disk is
attached successfully but boot index is set failed, we'll remove the
new disk to restore.
Signed-off-by: Wang Rui <moon.wangrui(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com>
---
src/qemu/qemu_hotplug.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 88 insertions(+)
If I read the tea leaves correctly - running this on < qemu 2.2 will
cause a failure since the ChangeBootIndex isn't supported there.
I don't get a sense why just failing the boot index setting should cause
the disk to not be attached. How do you differentiate the failure is
not supported vs. not present?
Seems to me there should be a way to qom-get and do some "pre-checks"
before adding, then trying to rip it out which is just ripe for all
sorts of errors. Taking that route perhaps means the code movement patch
is unnecessary and the device removal is unnecessary.
I didn't dive deep into the following code as I really think the qom-get
is something that'll obsolete certain parts of it.
John
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 2f84949..5eacfce 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2999,7 +2999,12 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
{
virDomainDiskDefPtr disk = dev->data.disk;
virDomainDiskDefPtr orig_disk = NULL;
+ virDomainDeviceDefPtr new_dev_copy = NULL;
+ virDomainDeviceDefPtr old_dev_copy = NULL;
+ virDomainDiskDefPtr tmp = NULL;
+ virCapsPtr caps = NULL;
int ret = -1;
+ int removed = 0;
const char *driverName = virDomainDiskGetDriver(disk);
const char *src = virDomainDiskGetSource(disk);
@@ -3022,6 +3027,13 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
goto end;
+ if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+ goto end;
+
+ if (!(new_dev_copy = virDomainDeviceDefCopy(dev, vm->def,
+ caps, driver->xmlopt)))
+ goto end;
+
switch (disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
@@ -3036,12 +3048,33 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
goto end;
}
+ tmp = dev->data.disk;
+ dev->data.disk = orig_disk;
+ /* save a copy of old device to restore */
+ if (!(old_dev_copy = virDomainDeviceDefCopy(dev, vm->def,
+ caps, driver->xmlopt))) {
+ dev->data.disk = tmp;
+ goto end;
+ }
+ dev->data.disk = tmp;
+
if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk,
disk->src, false) < 0)
goto end;
disk->src = NULL;
ret = 0;
+
+ tmp = new_dev_copy->data.disk;
+ if (orig_disk->info.bootIndex != tmp->info.bootIndex) {
+ /* 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, &orig_disk->info,
+ tmp->info.bootIndex ?
+ tmp->info.bootIndex : -1) < 0)
+ goto try_remove;
+ orig_disk->info.bootIndex = tmp->info.bootIndex;
+ }
break;
case VIR_DOMAIN_DISK_DEVICE_DISK:
@@ -3063,6 +3096,15 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
_("disk bus '%s' cannot be hotplugged."),
virDomainDiskBusTypeToString(disk->bus));
}
+
+ tmp = new_dev_copy->data.disk;
+ if (!ret && tmp->info.bootIndex > 0) {
+ if (qemuDomainChangeBootIndex(driver, vm, &disk->info,
+ tmp->info.bootIndex) < 0)
+ goto try_remove;
+ disk->info.bootIndex = tmp->info.bootIndex;
+ }
+
break;
default:
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
@@ -3074,7 +3116,53 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
end:
if (ret != 0)
ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name));
+ virObjectUnref(caps);
+ virDomainDeviceDefFree(new_dev_copy);
+ virDomainDeviceDefFree(old_dev_copy);
+ if (removed) {
+ dev->data.disk = NULL;
+ ret = -1;
+ }
return ret;
+
+ try_remove:
+ if (!virDomainObjIsActive(vm)) {
+ removed = 1;
+ goto end;
+ }
+
+ switch (new_dev_copy->data.disk->device) {
+ case VIR_DOMAIN_DISK_DEVICE_CDROM:
+ case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
+ if (qemuAddSharedDevice(driver, old_dev_copy, vm->def->name) < 0)
+ break;
+
+ tmp = old_dev_copy->data.disk;
+ if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk, tmp->src,
false) < 0) {
+ VIR_WARN("Unable to recover original media with target '%s' and
path '%s'",
+ tmp->dst, tmp->src->path);
+ ignore_value(qemuRemoveSharedDevice(driver, old_dev_copy,
vm->def->name));
+ } else {
+ tmp->src = NULL;
+ }
+ break;
+ case VIR_DOMAIN_DISK_DEVICE_DISK:
+ case VIR_DOMAIN_DISK_DEVICE_LUN:
+ if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
+ if (qemuDomainDetachVirtioDiskDevice(driver, vm, disk) != 0)
+ VIR_WARN("Unable to detach new disk with bus '%s' and
target '%s'",
+ virDomainDiskBusTypeToString(disk->bus), disk->dst);
+ }
+ else if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI ||
+ disk->bus == VIR_DOMAIN_DISK_BUS_USB)
+ if (qemuDomainDetachDiskDevice(driver, vm, disk) != 0)
+ VIR_WARN("Unable to detach new disk with bus '%s' and
target '%s'",
+ virDomainDiskBusTypeToString(disk->bus), disk->dst);
+ ret = -1;
+ break;
+ }
+ removed = 1;
+ goto end;
}
static int
7:42 p.m.
New subject: [libvirt] [PATCH 5/8] qemu: support attachment of disk device with boot index
On 01/05/2015 02:29 AM, Wang Rui wrote:
When we attach a disk to a running VM with boot index, we can 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. If new disk is
attached successfully but boot index is set failed, we'll remove the
new disk to restore.
Signed-off-by: Wang Rui <moon.wangrui(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com>
---
src/qemu/qemu_hotplug.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 88 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 2f84949..5eacfce 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2999,7 +2999,12 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
{
virDomainDiskDefPtr disk = dev->data.disk;
virDomainDiskDefPtr orig_disk = NULL;
+ virDomainDeviceDefPtr new_dev_copy = NULL;
+ virDomainDeviceDefPtr old_dev_copy = NULL;
+ virDomainDiskDefPtr tmp = NULL;
+ virCapsPtr caps = NULL;
int ret = -1;
+ int removed = 0;
const char *driverName = virDomainDiskGetDriver(disk);
const char *src = virDomainDiskGetSource(disk);
@@ -3022,6 +3027,13 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
goto end;
+ if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+ goto end;
+
+ if (!(new_dev_copy = virDomainDeviceDefCopy(dev, vm->def,
+ caps, driver->xmlopt)))
Format/alignment issue (caps under dev)
+ goto end;
+
switch (disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
@@ -3036,12 +3048,33 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
goto end;
}
+ tmp = dev->data.disk;
+ dev->data.disk = orig_disk;
+ /* save a copy of old device to restore */
+ if (!(old_dev_copy = virDomainDeviceDefCopy(dev, vm->def,
+ caps, driver->xmlopt))) {
+ dev->data.disk = tmp;
+ goto end;
+ }
+ dev->data.disk = tmp;
+
if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk,
disk->src, false) < 0)
goto end;
disk->src = NULL;
ret = 0;
+
+ tmp = new_dev_copy->data.disk;
+ if (orig_disk->info.bootIndex != tmp->info.bootIndex) {
+ /* 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, &orig_disk->info,
+ tmp->info.bootIndex ?
+ tmp->info.bootIndex : -1) < 0)
+ goto try_remove;
+ orig_disk->info.bootIndex = tmp->info.bootIndex;
+ }
break;
case VIR_DOMAIN_DISK_DEVICE_DISK:
@@ -3063,6 +3096,15 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
_("disk bus '%s' cannot be hotplugged."),
virDomainDiskBusTypeToString(disk->bus));
}
+
+ tmp = new_dev_copy->data.disk;
+ if (!ret && tmp->info.bootIndex > 0) {
+ if (qemuDomainChangeBootIndex(driver, vm, &disk->info,
+ tmp->info.bootIndex) < 0)
+ goto try_remove;
+ disk->info.bootIndex = tmp->info.bootIndex;
+ }
+
break;
default:
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
@@ -3074,7 +3116,53 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
end:
if (ret != 0)
ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name));
+ virObjectUnref(caps);
+ virDomainDeviceDefFree(new_dev_copy);
+ virDomainDeviceDefFree(old_dev_copy);
+ if (removed) {
+ dev->data.disk = NULL;
+ ret = -1;
+ }
return ret;
+
+ try_remove:
+ if (!virDomainObjIsActive(vm)) {
+ removed = 1;
+ goto end;
+ }
+
+ switch (new_dev_copy->data.disk->device) {
+ case VIR_DOMAIN_DISK_DEVICE_CDROM:
+ case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
+ if (qemuAddSharedDevice(driver, old_dev_copy, vm->def->name) < 0)
+ break;
+
+ tmp = old_dev_copy->data.disk;
+ if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk, tmp->src,
false) < 0) {
+ VIR_WARN("Unable to recover original media with target '%s' and
path '%s'",
+ tmp->dst, tmp->src->path);
+ ignore_value(qemuRemoveSharedDevice(driver, old_dev_copy,
vm->def->name));
+ } else {
+ tmp->src = NULL;
+ }
+ break;
+ case VIR_DOMAIN_DISK_DEVICE_DISK:
+ case VIR_DOMAIN_DISK_DEVICE_LUN:
+ if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
+ if (qemuDomainDetachVirtioDiskDevice(driver, vm, disk) != 0)
+ VIR_WARN("Unable to detach new disk with bus '%s' and
target '%s'",
+ virDomainDiskBusTypeToString(disk->bus), disk->dst);
+ }
+ else if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI ||
+ disk->bus == VIR_DOMAIN_DISK_BUS_USB)
+ if (qemuDomainDetachDiskDevice(driver, vm, disk) != 0)
+ VIR_WARN("Unable to detach new disk with bus '%s' and
target '%s'",
+ virDomainDiskBusTypeToString(disk->bus), disk->dst);
+ ret = -1;
+ break;
+ }
+ removed = 1;
+ goto end;
}
static int
3:27 a.m.
New subject: [libvirt] [PATCH 5/8] qemu: support attachment of disk device with boot index
On Mon, Jan 05, 2015 at 03:29:44PM +0800, Wang Rui wrote:
When we attach a disk to a running VM with boot index, we can 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).
It seems older QEMU silently ignores the bootindex specified with
device_add.
Checking for the support upfront lets libvirt error out earlier.
(Even in the case of updating the boot-index, there would be no need
to enter the monitor to report the error).
Can that be done by looking at some qom property, or can it only
be checked by comparing the QEMU version?
After this patch, the boot index will take effect after
virDomainAttachDevice(Flags) API returning success. If new disk is
attached successfully but boot index is set failed, we'll remove the
new disk to restore.
The bootindex can be specified as a part of the device_add command,
removing the need to detach the device on failure (and the previous code
movement patch).
Also, can the qom-set really fail? It seems the property has been
around for a while, it was just ignored.
Jan
Signed-off-by: Wang Rui <moon.wangrui(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com>
---
src/qemu/qemu_hotplug.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 88 insertions(+)
1:29 a.m.
New subject: [libvirt] [PATCH 6/8] conf: add compatiblity check for boot index when updating device
Signed-off-by: Wang Rui <moon.wangrui(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com>
---
src/conf/domain_conf.c | 43 +++++++++++++++++++++++--------------------
1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d2c4a0a..5beb830 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19991,29 +19991,32 @@ virDomainDefCompatibleDevice(virDomainDefPtr def,
{
virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev);
- if (action != VIR_DOMAIN_DEVICE_ACTION_ATTACH)
- return 0;
-
- if (!virDomainDefHasUSB(def) &&
- STRNEQ(def->os.type, "exe") &&
- virDomainDeviceIsUSB(dev)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Device configuration is not compatible: "
- "Domain has no USB bus support"));
- return -1;
- }
-
- if (info && info->bootIndex > 0) {
- if (def->os.nBootDevs > 0) {
+ switch (action) {
+ case VIR_DOMAIN_DEVICE_ACTION_ATTACH:
+ if (!virDomainDefHasUSB(def) &&
+ STRNEQ(def->os.type, "exe") &&
+ virDomainDeviceIsUSB(dev)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("per-device boot elements cannot be used"
- " together with os/boot elements"));
+ _("Device configuration is not compatible: "
+ "Domain has no USB bus support"));
return -1;
}
- if (virDomainDeviceInfoIterate(def,
- virDomainDeviceInfoCheckBootIndex,
- info) < 0)
- return -1;
+ case VIR_DOMAIN_DEVICE_ACTION_UPDATE:
+ if (info && info->bootIndex > 0) {
+ if (def->os.nBootDevs > 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("per-device boot elements cannot be used"
+ " together with os/boot elements"));
+ return -1;
+ }
+ if (virDomainDeviceInfoIterate(def,
+ virDomainDeviceInfoCheckBootIndex,
+ info) < 0)
+ return -1;
+ }
+ break;
+ default:
+ return 0;
}
return 0;
--
1.7.12.4
7:41 p.m.
New subject: [libvirt] [PATCH 6/8] conf: add compatiblity check for boot index when updating device
On 01/05/2015 02:29 AM, Wang Rui wrote:
Signed-off-by: Wang Rui <moon.wangrui(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com>
---
src/conf/domain_conf.c | 43 +++++++++++++++++++++++--------------------
1 file changed, 23 insertions(+), 20 deletions(-)
Since there's no commit message - even more reason to combine with patch
3 (or my 3b)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d2c4a0a..5beb830 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19991,29 +19991,32 @@ virDomainDefCompatibleDevice(virDomainDefPtr def,
{
virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev);
- if (action != VIR_DOMAIN_DEVICE_ACTION_ATTACH)
- return 0;
-
- if (!virDomainDefHasUSB(def) &&
- STRNEQ(def->os.type, "exe") &&
- virDomainDeviceIsUSB(dev)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Device configuration is not compatible: "
- "Domain has no USB bus support"));
- return -1;
- }
-
- if (info && info->bootIndex > 0) {
- if (def->os.nBootDevs > 0) {
+ switch (action) {
+ case VIR_DOMAIN_DEVICE_ACTION_ATTACH:
+ if (!virDomainDefHasUSB(def) &&
+ STRNEQ(def->os.type, "exe") &&
+ virDomainDeviceIsUSB(dev)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("per-device boot elements cannot be used"
- " together with os/boot elements"));
+ _("Device configuration is not compatible: "
+ "Domain has no USB bus support"));
return -1;
}
- if (virDomainDeviceInfoIterate(def,
- virDomainDeviceInfoCheckBootIndex,
- info) < 0)
- return -1;
+ case VIR_DOMAIN_DEVICE_ACTION_UPDATE:
+ if (info && info->bootIndex > 0) {
+ if (def->os.nBootDevs > 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("per-device boot elements cannot be used"
+ " together with os/boot elements"));
+ return -1;
+ }
+ if (virDomainDeviceInfoIterate(def,
+ virDomainDeviceInfoCheckBootIndex,
+ info) < 0)
+ return -1;
+ }
+ break;
+ default:
+ return 0;
The 'norm' of using default is to list all the cases - makes it easier
to determine when one is missing. I think the switch in this case is
overkill.
I think this is better:
if (action != VIR_DOMAIN_DEVICE_ACTION_ATTACH ||
action != VIR_DOMAIN_DEVICE_ACTION_UPDATE)
return 0;
if (action == VIR_DOMAIN_DEVICE_ACTION_ATTACH &&
!virDomainDefHasUSB(def) &&
STRNEQ(def->os.type, "exe") &&
virDomainDeviceIsUSB(dev)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Device configuration is not compatible: "
"Domain has no USB bus support"));
return -1;
}
if (info && info->bootIndex > 0) {
if (def->os.nBootDevs > 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("per-device boot elements cannot be used"
" together with os/boot elements"));
return -1;
}
if (virDomainDeviceInfoIterate(def,
virDomainDeviceInfoCheckBootIndex,
info) < 0)
return -1;
}
[sorry about the wrapping on the CheckBootIndex call - the point is all
you're doing is adding _UPDATE to the initial check... then adding the
"if (action == *_ATTACH)" for just the if USB check...
}
return 0;
1:29 a.m.
New subject: [libvirt] [PATCH 7/8] qemu: support updating interface with boot index
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(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com>
---
src/qemu/qemu_hotplug.c | 35 ++++++++++++++++++++++++++++++-----
1 file changed, 30 insertions(+), 5 deletions(-)
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
+ * 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.
--
1.7.12.4
7:41 p.m.
New subject: [libvirt] [PATCH 7/8] qemu: support updating interface with boot index
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(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)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.
1:29 a.m.
New subject: [libvirt] [PATCH 8/8] qemu: support updating disk with boot index
QEMU supported to set device's boot index online recently(since QEMU 2.2.0).
This patch implements the disk's boot index update lively.
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(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com>
---
src/qemu/qemu_driver.c | 64 +++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 55 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 673d8a6..9615fe4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7048,6 +7048,7 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
{
virDomainDiskDefPtr disk = dev->data.disk;
virDomainDiskDefPtr orig_disk = NULL;
+ int oldBootIndex = -1;
int ret = -1;
if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
@@ -7056,16 +7057,25 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
goto end;
+ if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def,
+ disk->bus, disk->dst))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("No device with bus '%s' and target
'%s'"),
+ virDomainDiskBusTypeToString(disk->bus),
+ disk->dst);
+ goto end;
+ }
+
switch (disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
- if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def,
- disk->bus, disk->dst))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("No device with bus '%s' and target
'%s'"),
- virDomainDiskBusTypeToString(disk->bus),
- disk->dst);
- goto end;
+ if (orig_disk->info.bootIndex != disk->info.bootIndex) {
+ oldBootIndex = orig_disk->info.bootIndex;
+ if (qemuDomainChangeBootIndex(driver, vm, &orig_disk->info,
+ disk->info.bootIndex ?
+ disk->info.bootIndex : -1) < 0)
+ goto end;
+ orig_disk->info.bootIndex = disk->info.bootIndex;
}
/* Add the new disk src into shared disk hash table */
@@ -7075,6 +7085,16 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
if (qemuDomainChangeEjectableMedia(driver, conn, vm,
orig_disk, dev->data.disk->src, force)
< 0) {
ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk,
vm->def->name));
+
+ /* Change media failed. Boot index should be changed back to original. */
+ if (oldBootIndex >= 0) {
+ if (qemuDomainChangeBootIndex(driver, vm, &orig_disk->info,
+ oldBootIndex ? oldBootIndex : -1) < 0)
{
+ VIR_WARN("Change Boot index back to original failed");
+ goto end;
+ }
+ orig_disk->info.bootIndex = oldBootIndex;
+ }
goto end;
}
@@ -7082,6 +7102,22 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
ret = 0;
break;
+ case VIR_DOMAIN_DISK_DEVICE_DISK:
+ if (orig_disk->info.bootIndex != disk->info.bootIndex) {
+ if (qemuDomainChangeBootIndex(driver, vm, &orig_disk->info,
+ disk->info.bootIndex ?
+ disk->info.bootIndex : -1) < 0)
+ goto end;
+ orig_disk->info.bootIndex = disk->info.bootIndex;
+ ret = 0;
+ break;
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("disk bus '%s' cannot be updated except boot
index."),
+ virDomainDiskBusTypeToString(disk->bus));
+ goto end;
+ }
+
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk bus '%s' cannot be updated."),
@@ -7413,8 +7449,17 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
return -1;
}
orig = vmdef->disks[pos];
- if (!(orig->device == VIR_DOMAIN_DISK_DEVICE_CDROM) &&
- !(orig->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)) {
+ if (orig->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
+ if (orig->info.bootIndex != disk->info.bootIndex) {
+ orig->info.bootIndex = disk->info.bootIndex;
+ break;
+ } else {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("this disk doesn't support update except boot
index"));
+ return -1;
+ }
+ } else if (!(orig->device == VIR_DOMAIN_DISK_DEVICE_CDROM) &&
+ !(orig->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
_("this disk doesn't support update"));
return -1;
@@ -7436,6 +7481,7 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
orig->src->format = disk->src->format;
disk->src->path = NULL;
orig->startupPolicy = disk->startupPolicy;
+ orig->info.bootIndex = disk->info.bootIndex;
break;
case VIR_DOMAIN_DEVICE_GRAPHICS:
--
1.7.12.4
7:41 p.m.
New subject: [libvirt] [PATCH 8/8] qemu: support updating disk with boot index
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 disk's boot index update lively.
Description needs some more beef... The usage of "update lively" doesn't
translate well. I think you're talking about being able to make an
'online update' of a disk's bootIndex. Essentially this allows us to
change an existing from "2" to "3" perhaps and then add a new
"2" - is
that correct? What happens if you have 1, 2, 3 ... try to change 3 to
2 - will be an error? The clearing of index in order to remove it needs
to be made clearer on the formatdomain.html.in and/or virsh.pod.
All this needs to be done via XML editing - prone to errors... Perhaps
consider adding a virsh option... Makes it a whole lot easier to adjust
things rather than attempting to generate XML to modify things. Trying
to figure out the magic incantation required to "remove" the index
"live" by only providing some XML to a virsh command can be a challenge.
Sure you understand it and I'm beginning to, but how would one "use"
this feature is not described anywhere.
John
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(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com>
---
src/qemu/qemu_driver.c | 64 +++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 55 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 673d8a6..9615fe4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7048,6 +7048,7 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
{
virDomainDiskDefPtr disk = dev->data.disk;
virDomainDiskDefPtr orig_disk = NULL;
+ int oldBootIndex = -1;
int ret = -1;
if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
@@ -7056,16 +7057,25 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
goto end;
+ if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def,
+ disk->bus, disk->dst))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("No device with bus '%s' and target
'%s'"),
+ virDomainDiskBusTypeToString(disk->bus),
+ disk->dst);
+ goto end;
+ }
+
switch (disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
- if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def,
- disk->bus, disk->dst))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("No device with bus '%s' and target
'%s'"),
- virDomainDiskBusTypeToString(disk->bus),
- disk->dst);
- goto end;
+ if (orig_disk->info.bootIndex != disk->info.bootIndex) {
+ oldBootIndex = orig_disk->info.bootIndex;
+ if (qemuDomainChangeBootIndex(driver, vm, &orig_disk->info,
+ disk->info.bootIndex ?
+ disk->info.bootIndex : -1) < 0)
+ goto end;
+ orig_disk->info.bootIndex = disk->info.bootIndex;
}
/* Add the new disk src into shared disk hash table */
@@ -7075,6 +7085,16 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
if (qemuDomainChangeEjectableMedia(driver, conn, vm,
orig_disk, dev->data.disk->src, force)
< 0) {
ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk,
vm->def->name));
+
+ /* Change media failed. Boot index should be changed back to original. */
+ if (oldBootIndex >= 0) {
+ if (qemuDomainChangeBootIndex(driver, vm, &orig_disk->info,
+ oldBootIndex ? oldBootIndex : -1) < 0)
{
+ VIR_WARN("Change Boot index back to original failed");
+ goto end;
+ }
+ orig_disk->info.bootIndex = oldBootIndex;
+ }
goto end;
}
@@ -7082,6 +7102,22 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
ret = 0;
break;
+ case VIR_DOMAIN_DISK_DEVICE_DISK:
+ if (orig_disk->info.bootIndex != disk->info.bootIndex) {
+ if (qemuDomainChangeBootIndex(driver, vm, &orig_disk->info,
+ disk->info.bootIndex ?
+ disk->info.bootIndex : -1) < 0)
+ goto end;
+ orig_disk->info.bootIndex = disk->info.bootIndex;
+ ret = 0;
+ break;
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("disk bus '%s' cannot be updated except boot
index."),
+ virDomainDiskBusTypeToString(disk->bus));
+ goto end;
+ }
+
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk bus '%s' cannot be updated."),
@@ -7413,8 +7449,17 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
return -1;
}
orig = vmdef->disks[pos];
- if (!(orig->device == VIR_DOMAIN_DISK_DEVICE_CDROM) &&
- !(orig->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)) {
+ if (orig->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
+ if (orig->info.bootIndex != disk->info.bootIndex) {
+ orig->info.bootIndex = disk->info.bootIndex;
+ break;
+ } else {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("this disk doesn't support update except boot
index"));
+ return -1;
+ }
+ } else if (!(orig->device == VIR_DOMAIN_DISK_DEVICE_CDROM) &&
+ !(orig->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
_("this disk doesn't support update"));
return -1;
@@ -7436,6 +7481,7 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
orig->src->format = disk->src->format;
disk->src->path = NULL;
orig->startupPolicy = disk->startupPolicy;
+ orig->info.bootIndex = disk->info.bootIndex;
break;
case VIR_DOMAIN_DEVICE_GRAPHICS:
3580
days inactive
3609
days old
18 comments
3 participants
participants (3)
-
John Ferlan -
Ján Tomko -
Wang Rui