[libvirt] [PATCH v2 2/2] Implement SCSI disk unplugging
With the introduction of the generic qemu device model, unplugging
SCSI disks works like a charm, so support it in libvirt.
* src/qemu/qemu_driver.c: Add qemudDomainDetachSCSIDiskDevice() to do the
unplugging, extend qemudDomainDetachDeviceAdd().
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
---
src/qemu/qemu_driver.c | 60 +--
1 files changed, 57 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 63ca57c..7321960 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7940,6 +7940,51 @@ static int qemudDomainDetachPciDiskDevice(struct
qemud_driver *driver,
qemudShrinkDisks(vm-def, i);
+if (driver-securityDriver
+driver-securityDriver-domainRestoreSecurityImageLabel
+driver-securityDriver-domainRestoreSecurityImageLabel(vm,
dev-data.disk) 0)
+VIR_WARN(Unable to restore security label on %s,
dev-data.disk-src);
+
+ret = 0;
+
+cleanup:
+return ret;
+}
+
+static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev,
+ unsigned long long qemuCmdFlags)
+{
+int i, ret = -1;
+virDomainDiskDefPtr detach = NULL;
+qemuDomainObjPrivatePtr priv = vm-privateData;
+
+i = qemudFindDisk(vm-def, dev-data.disk-dst);
+
+if (i 0) {
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+_(disk %s not found), dev-data.disk-dst);
+goto cleanup;
+}
+
+if (!(qemuCmdFlags QEMUD_CMD_FLAG_DEVICE)) {
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+ _(Underlying qemu does not support SCSI disk removal));
+goto cleanup;
+}
+
+detach = vm-def-disks[i];
+
+qemuDomainObjEnterMonitorWithDriver(driver, vm);
+if (qemuMonitorDelDevice(priv-mon, detach-info.alias) 0) {
+qemuDomainObjExitMonitor(vm);
+goto cleanup;
+}
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+qemudShrinkDisks(vm-def, i);
+
virDomainDiskDefFree(detach);
if (driver-securityDriver
@@ -8393,9 +8438,18 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
goto endjob;
if (dev-type == VIR_DOMAIN_DEVICE_DISK
-dev-data.disk-device == VIR_DOMAIN_DISK_DEVICE_DISK
-dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
-ret = qemudDomainDetachPciDiskDevice(driver, vm, dev, qemuCmdFlags);
+dev-data.disk-device == VIR_DOMAIN_DISK_DEVICE_DISK) {
+if (dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
+ret = qemudDomainDetachPciDiskDevice(driver, vm, dev,
qemuCmdFlags);
+}
+else if (dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_SCSI) {
+ret = qemudDomainDetachSCSIDiskDevice(driver, vm, dev,
+ qemuCmdFlags);
+}
+else {
+qemuReportError(VIR_ERR_NO_SUPPORT,
+%s, _(This type of disk cannot be hot unplugged));
+}
} else if (dev-type == VIR_DOMAIN_DEVICE_NET) {
ret = qemudDomainDetachNetDevice(driver, vm, dev, qemuCmdFlags);
} else if (dev-type == VIR_DOMAIN_DEVICE_CONTROLLER) {
--
1.6.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix SCSI disk unplugging
Daniel P. Berrange wrote: On Tue, May 04, 2010 at 05:18:24PM +0200, Wolfgang Mauerer wrote: Detaching disk devices is not only possible for VIR_DOMAIN_DISK_BUS_VIRTIO, but also for VIR_DOMAIN_DISK_BUS_SCSI, so take care of this possibility. Additionally, when the new-style device syntax is used, we do not need to check if the PCI address is valid since we don't need it to do the hot-unplugging. And while we're at it, drop the pci part in qemudDomainDetachPciDiskDevice() -- it's misleading since we do not necessarily have to deal with PCI addresses. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com --- src/qemu/qemu_driver.c | 16 +--- 1 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 704f824..f2b8517 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7862,10 +7862,10 @@ cleanup: } -static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, - virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - unsigned long long qemuCmdFlags) +static int qemudDomainDetachDiskDevice(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + unsigned long long qemuCmdFlags) I'd rather we introduced a separate method qemudDomainDetachSCSIDiskDevice since logically these are different types of objects/operations, that just happen to share a common monitor command with new QEMU. It also needs to give a more explicit error in the case where QEMUD_CMD_FLAG_DEVICE is not set for SCSI, since we can't support that for SCSI, but can for VirtIO. okay, I've created a separate function for SCSI disk unplug. Since the differences between VirtIO and SCSI disk unplug are tiny, I've tried to extract at least a few commonalities -- see the two follow-up patches. Albeit there's much more duplicated code lurking in qemu_driver.c, it might make sense to look at this some day. Best, Wolfgang -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/2] Refactor disk unplugging
We can reuse some of the code for other purposes.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
---
src/qemu/qemu_driver.c | 56 ++-
1 files changed, 36 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 47ae52c..63ca57c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7867,6 +7867,36 @@ cleanup:
}
+static inline int qemudFindDisk(virDomainDefPtr def, char *dst)
+{
+int i;
+
+for (i = 0 ; i def-ndisks ; i++) {
+if (STREQ(def-disks[i]-dst, dst)) {
+return i;
+}
+}
+
+return -1;
+}
+
+static inline void qemudShrinkDisks(virDomainDefPtr def, int i)
+{
+if (def-ndisks 1) {
+memmove(def-disks + i,
+def-disks + i + 1,
+sizeof(*def-disks) *
+(def-ndisks - (i + 1)));
+def-ndisks--;
+if (VIR_REALLOC_N(def-disks, def-ndisks) 0) {
+/* ignore, harmless */
+}
+} else {
+VIR_FREE(def-disks);
+def-ndisks = 0;
+}
+}
+
static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDeviceDefPtr dev,
@@ -7876,19 +7906,16 @@ static int qemudDomainDetachPciDiskDevice(struct
qemud_driver *driver,
virDomainDiskDefPtr detach = NULL;
qemuDomainObjPrivatePtr priv = vm-privateData;
-for (i = 0 ; i vm-def-ndisks ; i++) {
-if (STREQ(vm-def-disks[i]-dst, dev-data.disk-dst)) {
-detach = vm-def-disks[i];
-break;
-}
-}
+i = qemudFindDisk(vm-def, dev-data.disk-dst);
-if (!detach) {
+if (i 0) {
qemuReportError(VIR_ERR_OPERATION_FAILED,
_(disk %s not found), dev-data.disk-dst);
goto cleanup;
}
+detach = vm-def-disks[i];
+
if (!virDomainDeviceAddressIsValid(detach-info,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
qemuReportError(VIR_ERR_OPERATION_FAILED, %s,
@@ -7911,19 +7938,8 @@ static int qemudDomainDetachPciDiskDevice(struct
qemud_driver *driver,
}
qemuDomainObjExitMonitorWithDriver(driver, vm);
-if (vm-def-ndisks 1) {
-memmove(vm-def-disks + i,
-vm-def-disks + i + 1,
-sizeof(*vm-def-disks) *
-(vm-def-ndisks - (i + 1)));
-vm-def-ndisks--;
-if (VIR_REALLOC_N(vm-def-disks, vm-def-ndisks) 0) {
-/* ignore, harmless */
-}
-} else {
-VIR_FREE(vm-def-disks);
-vm-def-ndisks = 0;
-}
+qemudShrinkDisks(vm-def, i);
+
virDomainDiskDefFree(detach);
if (driver-securityDriver
--
1.6.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix SCSI disk unplugging
Detaching disk devices is not only possible for VIR_DOMAIN_DISK_BUS_VIRTIO,
but also for VIR_DOMAIN_DISK_BUS_SCSI, so take care of this possibility.
Additionally, when the new-style device syntax is used, we do not need to
check if the PCI address is valid since we don't need it to do the
hot-unplugging. And while we're at it, drop the pci part
in qemudDomainDetachPciDiskDevice() -- it's misleading since we
do not necessarily have to deal with PCI addresses.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
---
src/qemu/qemu_driver.c | 16 +---
1 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 704f824..f2b8517 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7862,10 +7862,10 @@ cleanup:
}
-static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver,
- virDomainObjPtr vm,
- virDomainDeviceDefPtr dev,
- unsigned long long qemuCmdFlags)
+static int qemudDomainDetachDiskDevice(struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev,
+ unsigned long long qemuCmdFlags)
{
int i, ret = -1;
virDomainDiskDefPtr detach = NULL;
@@ -7884,7 +7884,8 @@ static int qemudDomainDetachPciDiskDevice(struct
qemud_driver *driver,
goto cleanup;
}
-if (!virDomainDeviceAddressIsValid(detach-info,
+if (!(qemuCmdFlags QEMUD_CMD_FLAG_DEVICE)
+!virDomainDeviceAddressIsValid(detach-info,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
qemuReportError(VIR_ERR_OPERATION_FAILED, %s,
_(device cannot be detached without a PCI address));
@@ -8373,8 +8374,9 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
if (dev-type == VIR_DOMAIN_DEVICE_DISK
dev-data.disk-device == VIR_DOMAIN_DISK_DEVICE_DISK
-dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
-ret = qemudDomainDetachPciDiskDevice(driver, vm, dev, qemuCmdFlags);
+(dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_VIRTIO ||
+ dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_SCSI)) {
+ret = qemudDomainDetachDiskDevice(driver, vm, dev, qemuCmdFlags);
} else if (dev-type == VIR_DOMAIN_DEVICE_NET) {
ret = qemudDomainDetachNetDevice(driver, vm, dev, qemuCmdFlags);
} else if (dev-type == VIR_DOMAIN_DEVICE_CONTROLLER) {
--
1.6.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemudDomainAttachSCSIDisk: handle empty controller list
Jim Meyering wrote:
Clang found something that might be a real bug.
I suspect that ...drive.controller will always be at least one,
it can - explanation below.
but we should not have to dive into the code trying to figure
that out. It's easier/better here just to handle the potential trouble:
clang saw that if it *was* zero, then the following for loop
would not be entered, and cont would not be initialized.
On the very next statement cont (uninitialized) would be dereferenced.
(...)
* src/qemu/qemu_driver.c (qemudDomainAttachSCSIDisk): Handle
the (theoretical) case of an empty controller list, so that
clang does not think the subsequent dereference of cont
would dereference an undefined variable (due to preceding
loop not iterating even once).
---
src/qemu/qemu_driver.c |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7f7c459..efb1857 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5671,18 +5671,24 @@ static int qemudDomainAttachSCSIDisk(struct
qemud_driver *driver,
(...)
if (!(drivestr = qemuBuildDriveStr(disk, 0, qemuCmdFlags)))
goto error;
+if (disk-info.addr.drive.controller = 0) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(no drive controller for %s), disk-dst);
+goto error;
+}
+
for (i = 0 ; i = disk-info.addr.drive.controller ; i++) {
(...)
disk-info.addr.drive.controller does not denote the number of
available controllers, but an index -- which can very well be zero,
and the loop is always entered. Besides, checking for 0 in
the test does not make sense since
_virDomainDeviceDriveAddress.controller is unsigned.
Since this commit breaks SCSI disk hotplug on controller 0,
please revert it.
Thanks, Wolfgang
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemudDomainAttachSCSIDisk: handle empty controller list
Jim Meyering wrote:
Daniel P. Berrange wrote:
On Mon, Mar 15, 2010 at 03:56:55PM +0100, Wolfgang Mauerer wrote:
Jim Meyering wrote:
Clang found something that might be a real bug.
I suspect that ...drive.controller will always be at least one,
it can - explanation below.
but we should not have to dive into the code trying to figure
that out. It's easier/better here just to handle the potential trouble:
clang saw that if it *was* zero, then the following for loop
would not be entered, and cont would not be initialized.
On the very next statement cont (uninitialized) would be dereferenced.
(...)
* src/qemu/qemu_driver.c (qemudDomainAttachSCSIDisk): Handle
the (theoretical) case of an empty controller list, so that
clang does not think the subsequent dereference of cont
would dereference an undefined variable (due to preceding
loop not iterating even once).
---
src/qemu/qemu_driver.c |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7f7c459..efb1857 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5671,18 +5671,24 @@ static int qemudDomainAttachSCSIDisk(struct
qemud_driver *driver,
(...)
if (!(drivestr = qemuBuildDriveStr(disk, 0, qemuCmdFlags)))
goto error;
+if (disk-info.addr.drive.controller = 0) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(no drive controller for %s), disk-dst);
+goto error;
+}
+
for (i = 0 ; i = disk-info.addr.drive.controller ; i++) {
(...)
disk-info.addr.drive.controller does not denote the number of
available controllers, but an index -- which can very well be zero,
and the loop is always entered. Besides, checking for 0 in
the test does not make sense since
_virDomainDeviceDriveAddress.controller is unsigned.
Since this commit breaks SCSI disk hotplug on controller 0,
please revert it.
Agreed, this is definitely broken.
Thanks. The patch below reverts it.
However, there's more to it than that.
The controller index, while technically unsigned, may
be derived from an expression like -1 / 7,
since virDomainDiskDefAssignAddress does this:
void
virDomainDiskDefAssignAddress(virDomainDiskDefPtr def)
{
int idx = virDiskNameToIndex(def-dst);
switch (def-bus) {
case VIR_DOMAIN_DISK_BUS_SCSI:
...
def-info.addr.drive.controller = idx / 7;
def-info.addr.drive.bus = 0;
def-info.addr.drive.unit = idx % 7;
break;
case VIR_DOMAIN_DISK_BUS_IDE:
...
case VIR_DOMAIN_DISK_BUS_FDC:
...
...
and virDiskNameToIndex may return -1.
And idx is then -1.
While we might have gotten lucky with the -1 - 0 mapping for the
.controller member, I doubt a .unit (also unsigned int) value that's
derived from -1 % 7 (not portable, btw) will be safe.
I will propose a patch to change the above function
to return an indication of success or failure
and update the few callers. They're all in
functions that already return success or failure,
so this is the only interface that would change.
completely agreed.
Thanks, Wolfgang
Here's the revert:
From 4d7423fc0e27e86c937dde1a5d62f3b7b6e49451 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 15 Mar 2010 16:43:23 +0100
Subject: [PATCH] Revert f5a6ce44ce8368d4183b69a31b77f67688d9af43
* src/qemu/qemu_driver.c (qemudDomainAttachSCSIDisk): The .controller
member is an index, and *may* be 0. Reported by Wolfgang Mauerer.
---
src/qemu/qemu_driver.c |6 --
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f8ab545..04344a8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6057,12 +6057,6 @@ static int qemudDomainAttachSCSIDisk(struct
qemud_driver *driver,
if (!(drivestr = qemuBuildDriveStr(disk, 0, qemuCmdFlags)))
goto error;
-if (disk-info.addr.drive.controller = 0) {
-qemuReportError(VIR_ERR_INTERNAL_ERROR,
-_(no drive controller for %s), disk-dst);
-goto error;
-}
-
for (i = 0 ; i = disk-info.addr.drive.controller ; i++) {
cont = qemuDomainFindOrCreateSCSIDiskController(driver, vm, i,
qemuCmdFlags);
if (!cont)
--
1.7.0.2.398.g10c2f
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] Always link with libgcrypt
... not just when using the remote driver: libvirt.c always depends on it. This unbreaks builds with --without-remote Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- src/Makefile.am |7 --- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 2051e5d..8f0fd30 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -334,8 +334,11 @@ libvirt_la_LIBADD += libvirt_driver.la libvirt_driver_la_SOURCES = $(DRIVER_SOURCES) libvirt_driver_la_CFLAGS = $(NUMACTL_CFLAGS) \ + $(GNUTLS_CFLAGS)\ -...@top_srcdir@/src/conf -libvirt_driver_la_LDFLAGS = $(NUMACTL_LIBS) +libvirt_driver_la_LDFLAGS = $(NUMACTL_LIBS) \ + $(GNUTLS_LIBS) + USED_SYM_FILES = libvirt_private.syms @@ -362,11 +365,9 @@ noinst_LTLIBRARIES += libvirt_driver_remote.la libvirt_la_LIBADD += libvirt_driver_remote.la endif libvirt_driver_remote_la_CFLAGS = \ - $(GNUTLS_CFLAGS)\ $(SASL_CFLAGS) \ -...@top_srcdir@/src/conf libvirt_driver_remote_la_LDFLAGS = \ - $(GNUTLS_LIBS) \ $(SASL_LIBS) if WITH_DRIVER_MODULES libvirt_driver_remote_la_LDFLAGS += -module -avoid-version -- 1.6.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] Fix data structure handling when controllers are attached.
We must not delete the data part when the operation succeeds
because it is required later on. The correct pattern to handle
the parsed representation of the device information on success
is dev-data.controller = NULL; virDomainDeviceDefFree(dev);,
which leaves the structure pointed at by data in memory.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/qemu/qemu_driver.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1586e35..5394ff5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6111,6 +6111,8 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
if (dev-data.controller-type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
ret = qemudDomainAttachPciControllerDevice(driver, vm,
dev-data.controller,
qemuCmdFlags);
+if (ret == 0)
+dev-data.controller = NULL;
} else {
qemuReportError(VIR_ERR_NO_SUPPORT,
_(disk controller bus '%s' cannot be
hotplugged.),
--
1.6.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/6] SCSI-Disk-Hotremove
[PATCH 0/6] SCSI-Disk-Hotremove This series adds support for hot-removing SCSI disks when qemu supports the drive/device model. It also contains fixes for SCSI controller hotremoval and the build process. Cheers, Wolfgang --- Siemens AG, Corporate Technology, CT T TC 4 Corporate Competence Centre Embedded Linux Makefile.am |7 +-- qemu/qemu_driver.c | 97 +++ qemu/qemu_monitor.c | 13 ++ qemu/qemu_monitor.h |3 + qemu/qemu_monitor_json.c | 24 +++ qemu/qemu_monitor_json.h |3 + qemu/qemu_monitor_text.c | 40 +++ qemu/qemu_monitor_text.h |3 + 8 files changed, 180 insertions(+), 10 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] Use device_del to remove SCSI controllers
...when the underlying qemu supports the drive/device model and the
controller has been added this way.
Signed-off-by: Wolfgang Mauerer woilfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/qemu/qemu_driver.c | 31 +--
src/qemu/qemu_monitor.c | 13 +
src/qemu/qemu_monitor.h |3 +++
src/qemu/qemu_monitor_json.c | 24
src/qemu/qemu_monitor_json.h |3 +++
src/qemu/qemu_monitor_text.c | 40
src/qemu/qemu_monitor_text.h |3 +++
7 files changed, 111 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8960ef8..d683b1c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6230,7 +6230,8 @@ cleanup:
static int qemudDomainDetachPciControllerDevice(struct qemud_driver *driver,
virDomainObjPtr vm,
-virDomainDeviceDefPtr dev)
+virDomainDeviceDefPtr dev,
+unsigned long long
qemuCmdFlags)
{
int i, ret = -1;
virDomainControllerDefPtr detach = NULL;
@@ -6259,11 +6260,23 @@ static int qemudDomainDetachPciControllerDevice(struct
qemud_driver *driver,
goto cleanup;
}
+if (qemuCmdFlags QEMUD_CMD_FLAG_DEVICE) {
+if (qemuAssignDeviceControllerAlias(detach) 0)
+goto cleanup;
+}
+
qemuDomainObjEnterMonitorWithDriver(driver, vm);
-if (qemuMonitorRemovePCIDevice(priv-mon,
- detach-info.addr.pci) 0) {
-qemuDomainObjExitMonitor(vm);
-goto cleanup;
+if (qemuCmdFlags QEMUD_CMD_FLAG_DEVICE) {
+if (qemuMonitorDelDevice(priv-mon, detach-info.alias)) {
+qemuDomainObjExitMonitor(vm);
+goto cleanup;
+}
+} else {
+if (qemuMonitorRemovePCIDevice(priv-mon,
+ detach-info.addr.pci) 0) {
+qemuDomainObjExitMonitor(vm);
+goto cleanup;
+}
}
qemuDomainObjExitMonitorWithDriver(driver, vm);
@@ -6513,6 +6526,7 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
const char *xml) {
struct qemud_driver *driver = dom-conn-privateData;
virDomainObjPtr vm;
+unsigned long long qemuCmdFlags;
virDomainDeviceDefPtr dev = NULL;
int ret = -1;
@@ -6540,6 +6554,10 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
if (dev == NULL)
goto endjob;
+if (qemudExtractVersionInfo(vm-def-emulator,
+NULL,
+qemuCmdFlags) 0)
+goto endjob;
if (dev-type == VIR_DOMAIN_DEVICE_DISK
dev-data.disk-device == VIR_DOMAIN_DISK_DEVICE_DISK
@@ -6549,7 +6567,8 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
ret = qemudDomainDetachNetDevice(driver, vm, dev);
} else if (dev-type == VIR_DOMAIN_DEVICE_CONTROLLER) {
if (dev-data.controller-type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
-ret = qemudDomainDetachPciControllerDevice(driver, vm, dev);
+ret = qemudDomainDetachPciControllerDevice(driver, vm, dev,
+ qemuCmdFlags);
} else {
qemuReportError(VIR_ERR_NO_SUPPORT,
_(disk controller bus '%s' cannot be
hotunplugged.),
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index b88532c..a4d2b89 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1318,6 +1318,19 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon,
return ret;
}
+int qemuMonitorDelDevice(qemuMonitorPtr mon,
+ const char *devicestr)
+{
+DEBUG(mon=%p, fd=%d device(del)=%s, mon, mon-fd, devicestr);
+int ret;
+
+if (mon-json)
+ret = qemuMonitorJSONDelDevice(mon, devicestr);
+else
+ret = qemuMonitorTextDelDevice(mon, devicestr);
+return ret;
+}
+
int qemuMonitorAddDevice(qemuMonitorPtr mon,
const char *devicestr)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 0ac3957..3e55236 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -290,6 +290,9 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon,
int qemuMonitorAddDevice(qemuMonitorPtr mon,
const char *devicestr);
+int qemuMonitorDelDevice(qemuMonitorPtr mon,
+ const char *devicestr);
+
int qemuMonitorAddDrive(qemuMonitorPtr mon,
const char *drivestr);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 7b45594..3a94dd0 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
[libvirt] [PATCH 6/6] Implement hotremove for SCSI disks
Recent qemu versions allow us to add disks dynamically into the system
via the drive_add/device_add mechanism. Removing them is now just a
matter of using device_del, and this patch adds the required bits
and pieces to libvirt.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/qemu/qemu_driver.c | 57
1 files changed, 57 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d683b1c..ecbb23d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5494,6 +5494,60 @@ qemuDomainFindOrCreateSCSIDiskController(struct
qemud_driver *driver,
}
+static int qemudDomainDetachSCSIDisk(struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev,
+ unsigned int qemuCmdFlags)
+{
+int i;
+qemuDomainObjPrivatePtr priv = vm-privateData;
+virDomainDiskDefPtr detach = NULL;
+
+if (!(qemuCmdFlags QEMUD_CMD_FLAG_DEVICE)) {
+return -1;
+}
+
+for (i = 0 ; i vm-def-ndisks ; i++) {
+if (STREQ(vm-def-disks[i]-dst, dev-data.disk-dst)) {
+detach = vm-def-disks[i];
+break;
+}
+}
+
+if (!detach) {
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+_(disk %s not found), dev-data.disk-dst);
+return -1;
+}
+
+qemuDomainObjEnterMonitorWithDriver(driver, vm);
+/* Note: drive_del does not exist, but device_del will
+ automatically erase the associated drive as well */
+if (qemuMonitorDelDevice(priv-mon, detach-info.alias)) {
+qemuDomainObjExitMonitor(vm);
+return -1;
+}
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+if (vm-def-ndisks 1) {
+memmove(vm-def-disks + i,
+vm-def-disks + i + 1,
+sizeof(*vm-def-disks) *
+(vm-def-ndisks - (i + 1)));
+vm-def-ndisks--;
+if (VIR_REALLOC_N(vm-def-disks, vm-def-ndisks) 0) {
+/* ignore, harmless */
+}
+} else {
+VIR_FREE(vm-def-disks);
+vm-def-ndisks = 0;
+}
+virDomainDiskDefFree(detach);
+
+return 0;
+}
+
+
static int qemudDomainAttachSCSIDisk(struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk,
@@ -6563,6 +6617,9 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
dev-data.disk-device == VIR_DOMAIN_DISK_DEVICE_DISK
dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
ret = qemudDomainDetachPciDiskDevice(driver, vm, dev);
+} else if(dev-type == VIR_DOMAIN_DEVICE_DISK
+ dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_SCSI) {
+ret = qemudDomainDetachSCSIDisk(driver, vm, dev, qemuCmdFlags);
} else if (dev-type == VIR_DOMAIN_DEVICE_NET) {
ret = qemudDomainDetachNetDevice(driver, vm, dev);
} else if (dev-type == VIR_DOMAIN_DEVICE_CONTROLLER) {
--
1.6.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] Tiny spelling fix
---
src/qemu/qemu_driver.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1e4b493..1586e35 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5475,7 +5475,7 @@ qemuDomainFindOrCreateSCSIDiskController(struct
qemud_driver *driver,
return cont;
}
-/* No SCSI controller present, for back compatability we
+/* No SCSI controller present, for backward compatibility we
* now hotplug a controller */
if (VIR_ALLOC(cont) 0) {
virReportOOMError();
--
1.6.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] Fix PCI address handling when controllers are deleted.
When a controller is not present in the system anymore, the PCI address
must be deleted from libvirt's hashtable because it can be re-used for
other purposes.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/qemu/qemu_driver.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5394ff5..8960ef8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6280,6 +6280,11 @@ static int qemudDomainDetachPciControllerDevice(struct
qemud_driver *driver,
VIR_FREE(vm-def-controllers);
vm-def-ncontrollers = 0;
}
+
+if (qemuDomainPCIAddressReleaseAddr(priv-pciaddrs, detach-info) 0) {
+VIR_WARN0(Unable to release PCI address on controller);
+}
+
virDomainControllerDefFree(detach);
ret = 0;
--
1.6.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/34] Add device addressing and disk controller support
Hi, On Fri, Jan 8, 2010 at 6:22 PM, Daniel P. Berrange berra...@redhat.com wrote: This series is a merge of two previous series I posted http://www.redhat.com/archives/libvir-list/2009-December/msg00232.html http://www.redhat.com/archives/libvir-list/2009-December/msg00392.html It accomplishes quite a lot of things, having major impact on the QEMU driver, hopefully all in a postive way :-) do you happen to have a git repository from which the patches can be pulled? This would ease testing quite a bit since it's become a rather voluminous series ;-) Thanks, Wolfgang In particular it does * Add standard XML syntax for addressing of PCI devices, and disk drives * Add support for disk controllers as a managed device in XML * Add support for disk controller hotplug/unplug * Convert everything over to use QEMU's -device flag where available * Add PCI addressing when using -device * Introduce a way to give every device a unique 'alias' name in the XML format I can't promise it works perfectly, but i've done quite alot of positive testing with it now believe all the back comptability stuff is working right. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/12] Standardized device addressing SCSI controller/disk hotplug
Daniel P. Berrange wrote: On Thu, Dec 10, 2009 at 10:22:20PM +, Daniel P. Berrange wrote: The XML for each address type looks like address type='pci' mode='static' domain='0x' bus='0x1e' slot='0x07' function='0x0'/ address type='usb' mode='dynamic' bus='007' dev='003'/ address type='drive' mode='dynamic' controller='1' bus='0' unit='5'/ The 'mode' attribute for any of them is allowed to be either 'static' or 'dynamic'. A static address is one specified by the end user when defining the XML, while a dynamic address is one automatically chosen by libvirt/QEMU every time a guest boots. The idea of static addresses is to allow management apps to guarentee that PCI device drive numbering never changes. This series does not actually implement static addressing for PCI yet, because it requires that we change the way we generate QEMU command line arguments. It does do static addressing for disks. I'm now wondering whether we actually truely need the 'dynamic' option. It already doesn't really make sense for disks. For PCI though I'm not sure anyone would ever want the dynamic mode, where addresses can change at every boot. I think I should just remove the 'mode' attribute here, and just have libvirt detect PCI addresses the first time, and keep them the same thereafter. Maybe there are two things to distinguish: If you read dynamic in the sense of I don't care which address the device/disk/whatever uses, and I don't want to specify it explicitely, then having the option is reasonable . I agree that it does not make any sense to have dynamic in the sense explicitely assign new addresses every time system boots, but that would not happen any way: Given the same set of devices, the underlying hypervisor should assign the same addresses. Your suggestion would essentially imply that an initial dynamic specification is converted into a static one after the first boot. You are only referring to the in-memory configuration of libvirt when you talk about subsequent boots, or are you also thinking about modifying the persistent configuration? The second thought is whether we should put the address data inside a device element, so it looked like device type='pci' address domain='0x' bus='0x1e' slot='0x07' function='0x0'/ /device The reason being, that I think it might be interesting to also add in the vendor / product info at a later date device type='pci' vendor id='0x8086'/ product id='0x1234'/ address domain='0x' bus='0x1e' slot='0x07' function='0x0'/ /device Can't certainly do any harm to keep the format extensible. Regards, Wolfgang -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [PATCH 0/12] Standardized device addressing SCSI controller/disk hotplug
Hi,
Daniel P. Berrange wrote:
This patch series is a combination of series done by
Wolfgang Mauerer to support proper SCSI drive hotplug
and new work by myself to introduce generic addressing
for all devices.
Wolfgang's most recent posting was
http://www.redhat.com/archives/libvir-list/2009-November/msg00574.html
http://www.redhat.com/archives/libvir-list/2009-November/msg00701.html
When testing that series I came across a few minor issues,
but more importantly it made me realize how important it is
that we introduce explicit device addressing in our XML format.
(...)
essentially, this all looks good to me - thanks for the extensions.
Unfortunately, I was not yet successful in fully testing the code
because I have some issues with the underlying qemu that prevent my
machines from booting correctly with recent qemu-kvms. However, I came
across a small issue: When certain PCI devices are not present in the
system, libvirt can crash at startup. The attached patch fixes this.
Best, Wolfgang
commit 9d183e9710f8a062c4a95f208e0c39652016f729
Author: Wolfgang Mauerer wolfgang.maue...@siemens.com
Date: Mon Dec 14 17:09:37 2009 +0100
Fix crashes at startup when certain devices are not present
At startup, libvirt tries to detect which devices are available by parsing the
output of info pci. However, some of the possible devices (like watchdogs)
might not be available, which must be taken into account by the code.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9b5fac6..5d63108 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1754,6 +1754,9 @@ qemuGetPCIDiskVendorProduct(virDomainDiskDefPtr def,
unsigned *vendor,
unsigned *product)
{
+if (!def)
+return -1;
+
switch (def-bus) {
case VIR_DOMAIN_DISK_BUS_VIRTIO:
*vendor = QEMU_PCI_VENDOR_REDHAT;
@@ -1772,7 +1775,7 @@ qemuGetPCINetVendorProduct(virDomainNetDefPtr def,
unsigned *vendor,
unsigned *product)
{
-if (!def-model)
+if (!def || !def-model)
return -1;
if (STREQ(def-model, ne2k_pci)) {
@@ -1803,6 +1806,9 @@ qemuGetPCIControllerVendorProduct(virDomainControllerDefPtr def,
unsigned *vendor,
unsigned *product)
{
+if (!def)
+return -1;
+
switch (def-type) {
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
*vendor = QEMU_PCI_VENDOR_LSI_LOGIC;
@@ -1833,6 +1839,9 @@ qemuGetPCIVideoVendorProduct(virDomainVideoDefPtr def,
unsigned *vendor,
unsigned *product)
{
+if (!def)
+return -1;
+
switch (def-type) {
case VIR_DOMAIN_VIDEO_TYPE_CIRRUS:
*vendor = QEMU_PCI_VENDOR_CIRRUS;
@@ -1860,6 +1869,9 @@ qemuGetPCISoundVendorProduct(virDomainSoundDefPtr def,
unsigned *vendor,
unsigned *product)
{
+if (!def)
+return -1;
+
switch (def-model) {
case VIR_DOMAIN_SOUND_MODEL_ES1370:
*vendor = QEMU_PCI_VENDOR_ENSONIQ;
@@ -1883,6 +1895,9 @@ qemuGetPCIWatchdogVendorProduct(virDomainWatchdogDefPtr def,
unsigned *vendor,
unsigned *product)
{
+if (!def)
+return -1;
+
switch (def-model) {
case VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB:
*vendor = QEMU_PCI_VENDOR_INTEL;
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [PATCH 0/12] Standardized device addressing SCSI controller/disk hotplug
Daniel P. Berrange wrote: On Tue, Dec 15, 2009 at 01:06:51PM +0100, Wolfgang Mauerer wrote: Hi, Daniel P. Berrange wrote: This patch series is a combination of series done by Wolfgang Mauerer to support proper SCSI drive hotplug and new work by myself to introduce generic addressing for all devices. Wolfgang's most recent posting was http://www.redhat.com/archives/libvir-list/2009-November/msg00574.html http://www.redhat.com/archives/libvir-list/2009-November/msg00701.html When testing that series I came across a few minor issues, but more importantly it made me realize how important it is that we introduce explicit device addressing in our XML format. (...) essentially, this all looks good to me - thanks for the extensions. Unfortunately, I was not yet successful in fully testing the code because I have some issues with the underlying qemu that prevent my machines from booting correctly with recent qemu-kvms. However, I came across a small issue: When certain PCI devices are not present in the system, libvirt can crash at startup. The attached patch fixes this. Hmm, which particular device did you see it crash on? These functions are only called with 'def' straight out of the virDomainDefPtr struct, (eg, vm-def-disks[INDEX]) and those are all supposed to be non-NULL at all times. So if one is NULL then I've introduced a bug somewhere else ! I guess most likely in the new controllers code. it crashed with a missing watchdog. I realise that the other checks are not strictly necessary when users stick to the calling conventions, but figured that one check too many in the startup phase doesn't do any harm and is better than a crash. Best regards, Wolfgang -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [PATCH 10/12] Implement SCSI controller hotplug/unplug for QEMU
Hi,
Daniel P. Berrange wrote:
From: Wolfgang Mauerer wolfgang.maue...@siemens.com
This patch allows for explicit hotplug/unplug of SCSI controllers.
Ordinarily this is not required, since QEMU/libvirt will attach
a new SCSI controller whenever one is required. Allowing explicit
hotplug of controllers though, enables the caller to specify a
static PCI address, instead of auto-assigning the next available
PCI slot. Or it will when we have static PCI addressing.
This patch is derived from Wolfgang Mauerer's disk controller
patch series.
* src/qemu/qemu_driver.c: Support hotplug unplug of SCSI
controllers
* src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h,
src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h,
src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Add
new API for attaching PCI SCSI controllers
(...)
+int qemuMonitorTextAttachPCIDiskController(qemuMonitorPtr mon,
+ const char *bus,
+ virDomainDevicePCIAddress
*guestAddr)
+{
+char *cmd = NULL;
+char *reply = NULL;
+int tryOldSyntax = 0;
+int ret = -1;
+
+try_command:
+if (virAsprintf(cmd, pci_add %s storage if=%s,
+(tryOldSyntax ? 0: pci_addr=auto), bus) 0) {
+virReportOOMError(NULL);
+goto cleanup;
+}
I just realised that qemu-kvm HEAD prohibits adding empty SCSI
controllers currently, they consider this to be a bug. I've asked
to revert the corresponding patch on qemu-devel, but if this does
not happen, we will need to devise some other mechanism.
Regards, Wolfgang
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Disk- and Controller Hotplug
Hi, Some time ago, Daniel P. Berrange wrote: On Tue, Nov 24, 2009 at 02:15:58PM +0100, Wolfgang Mauerer wrote: Daniel P. Berrange wrote: On Mon, Nov 23, 2009 at 02:15:06PM +0100, Wolfgang Mauerer wrote: Daniel P. Berrange wrote: Wolfgang Mauerer wrote: okay, to avoid flooding the mailing list with nearly identical patches, I've put the rebased versions into a repository at git://git.kiszka.org/libvirt.git queue I've been taking a closer look at this, and I think the final state of patch series as a whole looks good. The minor issue is that the intermediate patches don't compile, since they rely on compile fixes at the end of the series. Rather than ask you to re-format the patches yet again, I'm going to just merge the patches into a smaller set myself. yes, I've sacrificed individual compilability when I did the forward porting because I the amount of time I can dedicate to this right now is a bit restricted for various reasons. Thanks for doing the missing cleanups! I've got just one question I'd like to understand before doing this. On the disk element's new controller element, you are allowing two mutually exclusive ways of specifying the controller disk ... controller name=identifier bus=number unit=number/ /disk and disk ... controller pci_addr=addr bus=number unit=number/ /disk I think it is going to cause unneccessary pain for applications to have to cope with two different ways of specifying the same relationship. So my intention is to remove the 'pci_addr' variant, since that information can easily be got directly from the separate top level controller device itself Getting rid of pci_addr is perfectly okay. I've kept it in the current series for setups that don't provide controllers, but it's supposed to be eliminated by design. Just go ahead if you want to do this it already now ;-) On a side note, once we've applied this initial series, I think we'll also need to be automatically adding a controller element in all guest domains which have IDE or SCSI controllers present by default. This is going to also force us to hook into QEMU's 'info pci' monitor command to figure out their PCI address. This is long overdue though and needed for NIC Disk devices added at startup too, so this is good motivation :-) agreed. I can try to address this when I bring back hot-removal. Best regards, Wolfgang -- Siemens AG, Corporate Technology CT T DE IT 1 Corporate Competence Centre Embedded Linux -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Disk- and Controller Hotplug
Hi, Daniel P. Berrange wrote: On Mon, Nov 23, 2009 at 02:15:06PM +0100, Wolfgang Mauerer wrote: Daniel P. Berrange wrote: On Tue, Nov 17, 2009 at 12:53:31AM +0100, wolfgang.maue...@siemens.com wrote: this is the second revision of a patch series to improve disk hotadd support for libvirt. It focuses on the qemu backend, but is naturally designed to be compatible with other backends as well. The objective is two-fold: 1.) Split off controller from disk handling. This is done by introducing a new domain element controller that is used to describe disk controllers. Support for hotplugging such controllers is added. Support to reference controllers by name is also included. 2.) disks can now be associated with a specific controller; this is done by means of a controller subelemnt for disks. This patch addresses the questions that were raised during the review of the initial submission, massages the code by fixing some whitespace issues, gets static controller configurations to work, and adds documentation. Notice that in contrast to the first submission I did not include the patch that adds support for disk- and controller hot_remove_. Since the qemu codebase is still in bit of a flux wrt. the necessary patches required for this functionality, it will reappear some time later as a separate submission. What libvirt version / GIT changeset did you create these patches against ? The current libvirt QEMU driver code in GIT is quite different, so the patches here don't apply for me as is. sorry for the late reply, I could not access my eMail during the last couple of days. Patches are on top of 790f0b3057787bb64, I did not realise that this one was only in the middle of qemu refactoring, not at the end :-( Do you plan any more refactorings to the qemu base in the near future, and if yes, are these already available somewhere? I'd like to avoid another useless rebase... No, the monitor code for the QEMU driver is stable now. I'll only be adding extra functionality, not changing existing stuff, so it should be good to rebase against now. okay, to avoid flooding the mailing list with nearly identical patches, I've put the rebased versions into a repository at git://git.kiszka.org/libvirt.git queue Best regards, Wolfgang The following changes since commit 8f147d16f1ccdd1f2ce063823fcf961e7bf396a5: Daniel P. Berrange (1): Fix default disk type when parsing QEMU argv are available in the git repository at: git://git.kiszka.org/libvirt.git queue Wolfgang Mauerer (13): Fix help message Clarify documentation for private symbols Extend disk element with controller information Add new domain device: controller Add disk-based hotplugging for the qemu backend Drop qemudAttachPciDiskDevice Implement controller hotplugging Allow controller selection by ID Remove surprises in the semantics of disk-hotadd Factor out the method to get the PCI address of a controller for a given disk Extract qemu monitor parts for controller hotplug Extract monitor parts from the qemu interaction for disk hotplug Update documentation: Controller daemon/libvirtd.c|2 +- docs/formatdomain.html.in| 38 + docs/schemas/domain.rng | 160 -- src/conf/domain_conf.c | 314 +- src/conf/domain_conf.h | 62 - src/libvirt_private.syms |7 +- src/qemu/qemu_driver.c | 194 -- src/qemu/qemu_monitor_text.c | 199 ++ src/qemu/qemu_monitor_text.h |9 ++ 9 files changed, 925 insertions(+), 60 deletions(-) -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] Disk- and Controller Hotplug
Hi, this is the second revision of a patch series to improve disk hotadd support for libvirt. It focuses on the qemu backend, but is naturally designed to be compatible with other backends as well. The objective is two-fold: 1.) Split off controller from disk handling. This is done by introducing a new domain element controller that is used to describe disk controllers. Support for hotplugging such controllers is added. Support to reference controllers by name is also included. 2.) disks can now be associated with a specific controller; this is done by means of a controller subelemnt for disks. This patch addresses the questions that were raised during the review of the initial submission, massages the code by fixing some whitespace issues, gets static controller configurations to work, and adds documentation. Notice that in contrast to the first submission I did not include the patch that adds support for disk- and controller hot_remove_. Since the qemu codebase is still in bit of a flux wrt. the necessary patches required for this functionality, it will reappear some time later as a separate submission. Best, Wolfgang daemon/libvirtd.c|2 +- docs/formatdomain.html.in| 38 + docs/schemas/domain.rng | 158 - src/conf/domain_conf.c | 314 +- src/conf/domain_conf.h | 62 - src/libvirt_private.syms |7 +- src/qemu/qemu_driver.c | 184 +++-- src/qemu/qemu_monitor_text.c | 197 ++ src/qemu/qemu_monitor_text.h |7 + 9 files changed, 910 insertions(+), 59 deletions(-) -- Siemens AG, CT T DE Corporate Competence Centre Embedded Linux -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/13] Remove surprises in the semantics of disk-hotadd
From: Wolfgang Mauerer wolfgang.maue...@siemens.com
When a disk is added without an explicitly specified
controller as host, then try to find the first available
controller. If none exists, do not (as in previous versions)
add a new PCI controller device with the disk attached,
but bail out with an error. Notice that this patch changes
the behaviour as compared to older libvirt releases, as
has been discussed on the mailing list (see
http://thread.gmane.org/gmane.comp.emulators.libvirt/15860)
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/qemu/qemu_driver.c | 122 +++-
1 files changed, 68 insertions(+), 54 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d3395a7..4f647c4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4406,68 +4406,82 @@ try_command:
controller_specified = 1;
}
-if (controller_specified) {
-if (dev-data.disk-controller_name) {
-for (i = 0 ; i vm-def-ncontrollers ; i++) {
-if (STREQ(dev-data.disk-controller_name,
- vm-def-controllers[i]-name)) {
-break;
-}
-}
-
-if (i == vm-def-ncontrollers) {
-qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _(Controller does not exist));
-return -1;
-}
-
-domain = vm-def-controllers[i]-address-data.pci.domain;
-bus= vm-def-controllers[i]-address-data.pci.bus;
-slot = vm-def-controllers[i]-address-data.pci.slot;
-} else {
-domain = dev-data.disk-controller_pci_addr.domain;
-bus= dev-data.disk-controller_pci_addr.bus;
-slot = dev-data.disk-controller_pci_addr.slot;
-
-for (i = 0 ; i vm-def-ncontrollers ; i++) {
-if (domain ==
vm-def-controllers[i]-address-data.pci.domain
-bus== vm-def-controllers[i]-address-data.pci.bus
-slot == vm-def-controllers[i]-address-data.pci.slot)
-break;
-}
-
-if (i == vm-def-ncontrollers) {
-qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _(Controller does not exist));
-return -1;
+ if (!controller_specified) {
+ /* Find an appropriate controller for disk-hotadd. Notice that
+ the admissible controller types (SCSI, virtio) have already
+ been checked in qemudDomainAttachDevice. */
+ for (i = 0 ; i vm-def-ncontrollers ; i++) {
+ if (vm-def-controllers[i]-type == dev-data.disk-type)
+ break;
+ }
+
+ if (i == vm-def-ncontrollers) {
+ qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ _(Cannot find appropriate controller for hot-add));
+ return -1;
+ }
+
+ domain = vm-def-controllers[i]-address-data.pci.domain;
+ bus= vm-def-controllers[i]-address-data.pci.bus;
+ slot = vm-def-controllers[i]-address-data.pci.slot;
+ }
+
+ if (controller_specified dev-data.disk-controller_name) {
+ for (i = 0 ; i vm-def-ncontrollers ; i++) {
+ if (STREQ(dev-data.disk-controller_name,
+ vm-def-controllers[i]-name)) {
+ break;
}
- }
-
-if (dev-data.disk-bus_id != -1) {
-virBufferVSprintf(buf, ,bus=%d, dev-data.disk-bus_id);
}
-if (dev-data.disk-unit_id != -1) {
-virBufferVSprintf(buf, ,unit=%d, dev-data.disk-unit_id);
+ if (i == vm-def-ncontrollers) {
+ qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+_(Controller does not exist));
+ return -1;
+ }
+
+ domain = vm-def-controllers[i]-address-data.pci.domain;
+ bus= vm-def-controllers[i]-address-data.pci.bus;
+ slot = vm-def-controllers[i]-address-data.pci.slot;
+ } else if (controller_specified) {
+ domain = dev-data.disk-controller_pci_addr.domain;
+ bus= dev-data.disk-controller_pci_addr.bus;
+ slot = dev-data.disk-controller_pci_addr.slot;
+
+ for (i = 0 ; i vm-def-ncontrollers ; i++) {
+ if (domain == vm-def-controllers[i]-address-data.pci.domain
+ bus== vm-def-controllers[i]-address-data.pci.bus
+ slot == vm-def-controllers[i]-address-data.pci.slot)
+ break;
}
-bus_unit_string = virBufferContentAndReset(buf);
-
-VIR_DEBUG (%s: drive_add %d:%d:%d file=%s,if=%s%s, vm-def-name,
- domain, bus, slot, safe_path, type, bus_unit_string);
-ret = virAsprintf(cmd, drive_add %s%d:%d:%d file=%s,if=%s%s
[libvirt] [PATCH 01/13] Fix help message
From: Wolfgang Mauerer wolfgang.maue...@siemens.com The configuration file setting is overriden by -f or --config, but not with -c Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com --- daemon/libvirtd.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index daf06bc..5cedd17 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -2931,7 +2931,7 @@ libvirt management daemon:\n\ \n\ Default paths:\n\ \n\ -Configuration file (unless overridden by -c):\n\ +Configuration file (unless overridden by -f):\n\ SYSCONF_DIR /libvirt/libvirtd.conf\n\ \n\ Sockets (as root):\n\ -- 1.6.4 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/13] Add disk-based hotplugging for the qemu backend
From: Wolfgang Mauerer wolfgang.maue...@siemens.com
When disks are added to a qemu backend with attach-device,
not just the disk, but a complete new PCI controller with
the disk attached is brought into the system. This patch
implements a proper disk hotplugging scheme for qemu.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/conf/domain_conf.c | 33 ++-
src/conf/domain_conf.h |5 +-
src/libvirt_private.syms |3 +-
src/qemu/qemu_driver.c | 243 +-
4 files changed, 278 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4e2b84e..91115b6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -896,7 +896,6 @@ virDomainDiskDefParseXML(virConnectPtr conn,
char *driverType = NULL;
char *source = NULL;
char *target = NULL;
-char *controller = NULL;
char *controller_pci_addr = NULL;
char *bus_id = NULL;
char *unit_id = NULL;
@@ -953,7 +952,8 @@ virDomainDiskDefParseXML(virConnectPtr conn,
if (target
STRPREFIX(target, ioemu:))
memmove(target, target+6, strlen(target)-5);
-} else if ((controller == NULL)
+} else if ((controller_name == NULL
+controller_pci_addr == NULL)
(xmlStrEqual(cur-name, BAD_CAST controller))) {
controller_name = virXMLPropString(cur, name);
controller_pci_addr = virXMLPropString(cur, pci_addr);
@@ -1059,7 +1059,13 @@ virDomainDiskDefParseXML(virConnectPtr conn,
}
}
-if (controller) {
+if (controller_name controller_pci_addr) {
+virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+ _(Both controller ID and address specified!));
+goto error;
+}
+
+if (controller_name) {
def-controller_name = controller_name;
def-bus_id = -1;
@@ -1090,6 +1096,23 @@ virDomainDiskDefParseXML(virConnectPtr conn,
}
}
+/* TODO: Should we re-use devaddr for this purpose,
+ or is an extra field justified? */
+def-controller_pci_addr.domain = -1;
+def-controller_pci_addr.bus = -1;
+def-controller_pci_addr.slot = -1;
+
+if (controller_pci_addr
+sscanf(controller_pci_addr, %x:%x:%x,
+ def-controller_pci_addr.domain,
+ def-controller_pci_addr.bus,
+ def-controller_pci_addr.slot) 3) {
+virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(Unable to parse controller 'pciaddr'
parameter '%s'),
+ controller_pci_addr);
+goto error;
+}
+
VIR_DEBUG(Controller PCI addr for disk parsed as %x:%x:%x\n,
def-controller_pci_addr.domain,
def-controller_pci_addr.bus,
@@ -1101,6 +1124,7 @@ virDomainDiskDefParseXML(virConnectPtr conn,
_(Invalid bus type '%s' for floppy disk), bus);
goto error;
}
+
if (def-device != VIR_DOMAIN_DISK_DEVICE_FLOPPY
def-bus == VIR_DOMAIN_DISK_BUS_FDC) {
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
@@ -1126,6 +1150,9 @@ virDomainDiskDefParseXML(virConnectPtr conn,
goto error;
}
+VIR_DEBUG(Disk PCI address parsed as %d:%d:%d\n,
+ def-pci_addr.domain, def-pci_addr.bus, def-pci_addr.slot);
+
def-src = source;
source = NULL;
def-dst = target;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 591143f..b30d08c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -133,7 +133,7 @@ struct _virDomainDiskDef {
int device;
int bus; /* Bus type, e.g. scsi or ide */
int bus_id; /* Bus number on the controller */
-int unit_id; /* Unit on the controller */
+int unit_id; /* Unit on the controller (both -1 if unspecified) */
char *src;
char *dst;
char *controller_name;
@@ -744,6 +744,9 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def);
void virDomainHostdevDefFree(virDomainHostdevDefPtr def);
void virDomainDeviceDefFree(virDomainDeviceDefPtr def);
void virDomainDeviceAddressFree(virDomainDeviceAddressPtr def);
+int virDomainCompareDeviceAddresses(virDomainDeviceAddressPtr a,
+virDomainDeviceAddressPtr b);
+const char *virDomainFormatDeviceAddress(virDomainDeviceAddressPtr address);
void virDomainDefFree(virDomainDefPtr vm);
void virDomainObjFree(virDomainObjPtr vm);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 92cf86c..e73ed94 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -152,7 +152,8 @@ virDomainObjListGetActiveIDs;
virDomainObjListNumOfDomains;
virDomainObjListInit;
virDomainObjListDeinit
[libvirt] [PATCH 06/13] Drop qemudAttachPciDiskDevice
From: Wolfgang Mauerer wolfgang.maue...@siemens.com
This function is not necessary anymore
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/qemu/qemu_driver.c | 33 -
1 files changed, 0 insertions(+), 33 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9f71562..1fbfdde 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4597,39 +4597,6 @@ try_command:
return 0;
}
-static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
- virDomainObjPtr vm,
- virDomainDeviceDefPtr dev)
-{
-int i;
-const char* type = virDomainDiskBusTypeToString(dev-data.disk-bus);
-
-for (i = 0 ; i vm-def-ndisks ; i++) {
-if (STREQ(vm-def-disks[i]-dst, dev-data.disk-dst)) {
-qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _(target %s already exists), dev-data.disk-dst);
-return -1;
-}
-}
-
-if (VIR_REALLOC_N(vm-def-disks, vm-def-ndisks+1) 0) {
-virReportOOMError(conn);
-return -1;
-}
-
-if (qemuMonitorAddPCIDisk(vm,
- dev-data.disk-src,
- type,
- dev-data.disk-pci_addr.domain,
- dev-data.disk-pci_addr.bus,
- dev-data.disk-pci_addr.slot) 0)
-return -1;
-
-virDomainDiskInsertPreAlloced(vm-def, dev-data.disk);
-
-return 0;
-}
-
static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr conn,
virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
--
1.6.4
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/13] Extend disk element with controller information
From: Wolfgang Mauerer wolfgang.maue...@siemens.com
This allows us to connect a disk with a specific controller,
which is required for disk hotadd/remove. A new XML child
element is added to the disk container:
disk
...
controller name=identifier pci_addr=addr bus=number
unit=number/
/disk
Normally the name of the controller must be specified. Using pci_addr
instead is supported for qemu guests that don't define a controller
in their main configuration file and cannot hotplug controllers.
When the controller has been brought into the system via the hotplug
mechanism also included in this patch series, it will have an ID.
Wrt. the data structures touched in this commit, notice
that while bus as subelement of target specifies
the type of bus (e.g., scsi, ide,...), it is used
to enumerate the buses on the controller here.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/conf/domain_conf.c | 29 +
src/conf/domain_conf.h |5 -
2 files changed, 33 insertions(+), 1 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 28bee54..ecddb68 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -724,7 +724,12 @@ virDomainDiskDefParseXML(virConnectPtr conn,
char *driverType = NULL;
char *source = NULL;
char *target = NULL;
+char *controller = NULL;
+char *bus_id = NULL;
+char *unit_id = NULL;
+char *controller_id = NULL;
char *bus = NULL;
+char *unit = NULL;
char *cachetag = NULL;
char *devaddr = NULL;
virStorageEncryptionPtr encryption = NULL;
@@ -768,12 +773,18 @@ virDomainDiskDefParseXML(virConnectPtr conn,
(xmlStrEqual(cur-name, BAD_CAST target))) {
target = virXMLPropString(cur, dev);
bus = virXMLPropString(cur, bus);
+unit = virXMLPropString(cur, unit);
/* HACK: Work around for compat with Xen
* driver in previous libvirt releases */
if (target
STRPREFIX(target, ioemu:))
memmove(target, target+6, strlen(target)-5);
+} else if ((controller == NULL)
+ (xmlStrEqual(cur-name, BAD_CAST controller))) {
+ controller_id = virXMLPropString(cur, id);
+ bus_id = virXMLPropString(cur, bus);
+ unit_id = virXMLPropString(cur, unit);
} else if ((driverName == NULL)
(xmlStrEqual(cur-name, BAD_CAST driver))) {
driverName = virXMLPropString(cur, name);
@@ -874,6 +885,24 @@ virDomainDiskDefParseXML(virConnectPtr conn,
}
}
+if (controller) {
+def-controller_id = controller_id;
+
+def-bus_id = -1;
+ if (bus_id virStrToLong_i(bus_id, NULL, 10, def-bus_id) 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+_(Cannot parse controller 'bus' attribute));
+ goto error;
+ }
+
+ def-unit_id = -1;
+ if (unit_id virStrToLong_i(unit_id, NULL, 10, def-unit_id) 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+_(Cannot parse controller 'unit' attribute));
+ goto error;
+ }
+}
+
if (def-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY
def-bus != VIR_DOMAIN_DISK_BUS_FDC) {
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index fadf43f..c034ae4 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -107,9 +107,12 @@ typedef virDomainDiskDef *virDomainDiskDefPtr;
struct _virDomainDiskDef {
int type;
int device;
-int bus;
+int bus; /* Bus type, e.g. scsi or ide */
+int bus_id; /* Bus number on the controller */
+int unit_id; /* Unit on the controller */
char *src;
char *dst;
+char *controller_id;
char *driverName;
char *driverType;
char *serial;
--
1.6.4
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/13] Clarify documentation for private symbols
From: Wolfgang Mauerer wolfgang.maue...@siemens.com The instruction See Makefile.am in libvirt.private_syms always makes me think that this file is autogenerated and should not be touched manually. This patch spares every reader of libvirt.private_syms the hassle of reading Makefile.am before augmenting libvirt.private_syms. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- src/libvirt_private.syms |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 310376b..fb81ace 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1,5 +1,6 @@ # -# General private symbols. See Makefile.am. +# General private symbols. Add symbols here, and see Makefile.am for +# more details. # -- 1.6.4 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/13] Allow controller selection by ID
From: Wolfgang Mauerer wolfgang.maue...@siemens.com
... by simply traversing the list of controllers
to find the associated PCI address.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/qemu/qemu_driver.c | 37 +
1 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1b20937..d3395a7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4407,19 +4407,40 @@ try_command:
}
if (controller_specified) {
-/* NOTE: Proper check for the controller will be implemented
- in a later commit */
- domain = dev-data.disk-controller_pci_addr.domain;
- bus= dev-data.disk-controller_pci_addr.bus;
- slot = dev-data.disk-controller_pci_addr.slot;
-
if (dev-data.disk-controller_name) {
-/* TODO: Obtain the PCI address of the controller
- from the data structures using the ID */
+for (i = 0 ; i vm-def-ncontrollers ; i++) {
+if (STREQ(dev-data.disk-controller_name,
+ vm-def-controllers[i]-name)) {
+break;
+}
+}
+
+if (i == vm-def-ncontrollers) {
+qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ _(Controller does not exist));
+return -1;
+}
+
+domain = vm-def-controllers[i]-address-data.pci.domain;
+bus= vm-def-controllers[i]-address-data.pci.bus;
+slot = vm-def-controllers[i]-address-data.pci.slot;
} else {
domain = dev-data.disk-controller_pci_addr.domain;
bus= dev-data.disk-controller_pci_addr.bus;
slot = dev-data.disk-controller_pci_addr.slot;
+
+for (i = 0 ; i vm-def-ncontrollers ; i++) {
+if (domain ==
vm-def-controllers[i]-address-data.pci.domain
+bus== vm-def-controllers[i]-address-data.pci.bus
+slot == vm-def-controllers[i]-address-data.pci.slot)
+break;
+}
+
+if (i == vm-def-ncontrollers) {
+qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ _(Controller does not exist));
+return -1;
+}
}
if (dev-data.disk-bus_id != -1) {
--
1.6.4
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/13] Add new domain device: controller
From: Wolfgang Mauerer wolfgang.maue...@siemens.com This augments virDomainDevice with a controller element that is used to represent disk controllers (e.g., scsi controllers). The XML format is given by controller type=scsi name=my_id address type=pci domain=0xNUM bus=0xNUM slot=0xNUM/ /controller where type denotes the disk interface (scsi, ide,...), name is an arbitrary string that identifies the controller for disk hotadd/remove, and the address element specifies the controller address on the PCI bus. Domain, bus and slot are specified as hexadecimal numbers. The address element can be omitted; in this case, an address will be assigned automatically. Currently, only hotplugging scsi controllers on a PCI bus is supported, and this only for qemu guests Routines for parsing this definition and the associated data structures are included in this commit. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- docs/schemas/domain.rng | 158 ++-- src/conf/domain_conf.c | 279 +- src/conf/domain_conf.h | 48 +++- src/libvirt_private.syms |1 - src/qemu/qemu_driver.c |1 - src/qemu/qemu_monitor_text.c | 47 +++ 6 files changed, 488 insertions(+), 46 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index b75f17e..0d5fb47 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -393,49 +393,129 @@ define name=disk element name=disk optional -attribute name=device + attribute name=device + choice + valuefloppy/value + valuedisk/value + valuecdrom/value + /choice + /attribute + /optional + optional + element name=controller + optional + attribute name=bus + ref name=unsignedInt/ + /attribute + /optional + optional + attribute name=unit + ref name=unsignedInt/ + /attribute + /optional + choice + attribute name=name + ref name=genericName/ + /attribute + attribute name=pciaddr + !-- Just for testing -- + ref name=deviceName/ + /attribute + /choice + /element + /optional + choice + group + attribute name=type + valuefile/value + /attribute + interleave + optional + element name=source + attribute name=file + ref name=absFilePath/ + /attribute + empty/ + /element + /optional + ref name=diskspec/ + /interleave + /group + group + attribute name=type + valueblock/value + /attribute + interleave + optional + element name=source + attribute name=dev + ref name=deviceName/ + /attribute + empty/ + /element + /optional + ref name=diskspec/ + /interleave + /group + ref name=diskspec/ + /choice +/element + /define + define name=target +element name=target + attribute name=dev +ref name=deviceName/ + /attribute + optional +attribute name=bus choice -valuefloppy/value -valuedisk/value -valuecdrom/value +valueide/value +valuefdc/value +valuescsi/value +valuevirtio/value +valuexen/value +valueusb/value +valueuml/value /choice /attribute /optional - choice -group - attribute name=type -valuefile/value - /attribute - interleave -optional - element name=source -attribute name=file - ref name=absFilePath/ -/attribute -empty/ - /element -/optional -ref name=diskspec/ - /interleave -/group -group - attribute name=type -valueblock/value - /attribute - interleave -optional - element name=source -attribute name=dev - ref name=deviceName/ -/attribute -empty/ - /element -/optional -ref name=diskspec/ - /interleave -/group -ref name=diskspec/ - /choice +/element + /define + + define name=controller +element name=controller + interleave + attribute name=type + choice + !-- For now, only SCSI is supported -- + valuescsi/value + /choice + /attribute
[libvirt] [PATCH 07/13] Implement controller hotplugging
From: Wolfgang Mauerer wolfgang.maue...@siemens.com
This enables to hot-add disk controllers without attached
disks into the system. Previously, it was only possible to
(implicitly) add disk controllers in the static machine
configuration.
Notice that the actual functionality is only available
for qemu at present, but other emulators can be extended
likewise.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/conf/domain_conf.c |5 +
src/qemu/qemu_driver.c | 19 ++-
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 91115b6..e90975f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -880,7 +880,6 @@ error:
goto cleanup;
}
-
/* Parse the XML definition for a disk
* @param node XML nodeset to parse for disk definition
*/
@@ -2940,7 +2939,6 @@ virDomainDeviceDefPtr
virDomainDeviceDefParse(virConnectPtr conn,
}
#endif
-
int virDomainDiskInsert(virDomainDefPtr def,
virDomainDiskDefPtr disk)
{
@@ -3326,8 +3324,7 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr
conn,
def-controllers[def-ncontrollers++] = controller;
}
-/*qsort(def-controllers, def-ncontrollers, sizeof(*def-controllers),
- virDomainControllerQSort); */
+
VIR_FREE(nodes);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1fbfdde..1b20937 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4413,6 +4413,15 @@ try_command:
bus= dev-data.disk-controller_pci_addr.bus;
slot = dev-data.disk-controller_pci_addr.slot;
+if (dev-data.disk-controller_name) {
+/* TODO: Obtain the PCI address of the controller
+ from the data structures using the ID */
+} else {
+domain = dev-data.disk-controller_pci_addr.domain;
+bus= dev-data.disk-controller_pci_addr.bus;
+slot = dev-data.disk-controller_pci_addr.slot;
+ }
+
if (dev-data.disk-bus_id != -1) {
virBufferVSprintf(buf, ,bus=%d, dev-data.disk-bus_id);
}
@@ -4568,6 +4577,8 @@ try_command:
VIR_FREE(cmd);
+/* Naturally, the controller hotplug reply is identical with
+ any other PCI hotplug reply */
if (qemudParsePciAddReply(vm, reply, domain, bus, slot) 0) {
if (!tryOldSyntax strstr(reply, invalid char in expression)) {
VIR_FREE(reply);
@@ -4582,18 +4593,14 @@ try_command:
}
/* Also fill in when the address was explicitely specified in
- case qemu changed it (TODO: Can this really happen?) */
+ case qemu changed it */
dev-data.controller-address-data.pci.domain = domain;
dev-data.controller-address-data.pci.bus= bus;
dev-data.controller-address-data.pci.slot = slot;
VIR_FREE(reply);
-/* NOTE: Sort function is added in a later commit */
vm-def-controllers[vm-def-ncontrollers++] = dev-data.controller;
-/*qsort(vm-def-disks, vm-def-ndisks, sizeof(*vm-def-disks),
-virDomainDiskQSort);*/
-
return 0;
}
@@ -4947,6 +4954,8 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
virCgroupDenyDevicePath(cgroup,
dev-data.disk-src);
}
+} else if (dev-type == VIR_DOMAIN_DEVICE_CONTROLLER) {
+ret = qemudDomainAttachDiskController(dom-conn, vm, dev);
} else if (dev-type == VIR_DOMAIN_DEVICE_NET) {
ret = qemudDomainAttachNetDevice(dom-conn, driver, vm, dev,
qemuCmdFlags);
} else if (dev-type == VIR_DOMAIN_DEVICE_HOSTDEV) {
--
1.6.4
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 13/13] Update documentation: Controller
From: Wolfgang Mauerer wolfgang.maue...@siemens.com Document the controller specification, that is, the controller element within domain --- docs/formatdomain.html.in | 38 ++ 1 files changed, 38 insertions(+), 0 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 52889af..aada7d9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -345,6 +345,7 @@ lt;disk type='file'gt; lt;driver name=tap type=aiogt; lt;source file='/var/lib/xen/images/fv0'/gt; +lt;controller name='controller0' bus='0' unit='5'gt; lt;target dev='hda' bus='ide'/gt; lt;encryption type='...'gt; ... @@ -366,6 +367,13 @@ specifies the fully-qualified path to the file holding the disk. If the disk codetype/code is block, then the codedev/code attribute specifies the path to the host device to serve as the disk. span class=sinceSince 0.0.3/span/dd + dtcodecontroller/code/dt + ddIf present, specifies the controller to which the disk is + connected. This is useful for disk hotplug as it allows to add + disks to specific controlers. This is currently only possible + for scsi drivers. codebus/code and codeunit/unit are + facultative and specify the drive position on the controller. + /dd dtcodetarget/code/dt ddThe codetarget/code element controls the bus / device under which the disk is exposed to the guest OS. The codedev/code attribute indicates @@ -675,6 +683,36 @@ qemu-kvm -net nic,model=? /dev/null It takes values xen (paravirtualized), ps2 and usb./dd /dl +h4a name=elementsControllerDisk controllers/a/h4 + +p +Disk controllers are hosts for hard disks. +/p + +pre + ... + lt;controller type='scsi' name='controller0'gt; + lt;address type='pci' bus='0x00' domain='0x00' slot='0x0a'/gt; + lt;/controllergt; + .../pre + +dl + dtcodecontroller/code/dt + ddThe codecontroller/code element has a mandatory codetype/code + attribute which takes the type of the controller, + e.g., codescsi/code. br/ + codename/code specifies a symbolic identifier for the + controller by which it can be addressed. + + The address of the controller on the underlying bus is given + by the codeaddress/code subelement. The mandatory + element codetype/code specified the bus type for which + the address holds and must be the same as for the controller. + Currently, only pci is supported, which requires three + parameters to denote address: codebus/code, codedomain/code, + and codedevice/code. All must be given as hexadecimal numbers. + /dd +/dl h4a name=elementsGraphicsGraphical framebuffers/a/h4 -- 1.6.4 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/13] Extract qemu monitor parts for controller hotplug
From: Wolfgang Mauerer wolfgang.maue...@siemens.com
This separates the communication with qemu a bit from the more
libvirtish actions.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/qemu/qemu_driver.c | 61 +--
src/qemu/qemu_monitor_text.c | 65 ++
src/qemu/qemu_monitor_text.h |2 +
3 files changed, 69 insertions(+), 59 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9f44685..289c3c6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4580,12 +4580,7 @@ static int qemudDomainAttachDiskController(virConnectPtr
conn,
virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
{
-int ret, i;
-char *cmd, *reply;
-char *tmp;
-const char* type =
virDomainDiskBusTypeToString(dev-data.controller-type);
-int tryOldSyntax = 0;
-unsigned domain, bus, slot;
+int i;
/* Only SCSI controllers are supported at the moment */
if (dev-data.controller-address
@@ -4618,59 +4613,7 @@ static int qemudDomainAttachDiskController(virConnectPtr
conn,
return -1;
}
-try_command:
-if (dev-data.controller-address != NULL) {
-domain = dev-data.controller-address-data.pci.domain;
-bus= dev-data.controller-address-data.pci.bus;
-slot = dev-data.controller-address-data.pci.slot;
-
- ret = virAsprintf(tmp, %.2x:%.2x:%.2x, domain, bus, slot);
-ret |= virAsprintf(cmd, pci_add %s%s storage if=%s,
- (tryOldSyntax ? : pci_addr=), tmp, type);
-} else {
-ret = virAsprintf(cmd, pci_add %s storage if=%s,
- (tryOldSyntax ? 0 : pci_addr=auto), type);
-}
-
-if (ret == -1) {
-virReportOOMError(conn);
-return ret;
-}
-
-if (qemudMonitorCommand(vm, cmd, reply) 0) {
-qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _(cannot attach %s controller), type);
-VIR_FREE(cmd);
-return -1;
-}
-
-VIR_FREE(cmd);
-
-/* Naturally, the controller hotplug reply is identical with
- any other PCI hotplug reply */
-if (qemudParsePciAddReply(vm, reply, domain, bus, slot) 0) {
-if (!tryOldSyntax strstr(reply, invalid char in expression)) {
- VIR_FREE(reply);
- tryOldSyntax = 1;
- goto try_command;
- }
-
- qemudReportError (conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _(adding %s controller failed: %s), type, reply);
- VIR_FREE(reply);
- return -1;
-}
-
-/* Also fill in when the address was explicitely specified in
- case qemu changed it */
-dev-data.controller-address-data.pci.domain = domain;
-dev-data.controller-address-data.pci.bus= bus;
-dev-data.controller-address-data.pci.slot = slot;
-
-VIR_FREE(reply);
-
-vm-def-controllers[vm-def-ncontrollers++] = dev-data.controller;
-return 0;
+return qemuMonitorAttachDiskController(vm, dev);
}
static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr conn,
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index f6f3e58..7326d05 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -1529,6 +1529,71 @@ qemudParseDriveAddReply(virDomainObjPtr vm,
return 0;
}
+int qemuMonitorAttachDiskController(virDomainObjPtr vm,
+virDomainDeviceDefPtr dev)
+{
+int ret;
+char *cmd, *reply;
+char *tmp;
+const char* type =
virDomainDiskBusTypeToString(dev-data.controller-type);
+int tryOldSyntax = 0;
+unsigned domain, bus, slot;
+
+try_command:
+if (dev-data.controller-address != NULL) {
+domain = dev-data.controller-address-data.pci.domain;
+ bus= dev-data.controller-address-data.pci.bus;
+ slot = dev-data.controller-address-data.pci.slot;
+
+ ret = virAsprintf(tmp, %.2x:%.2x:%.2x, domain, bus, slot);
+ret |= virAsprintf(cmd, pci_add %s%s storage if=%s,
+ (tryOldSyntax ? : pci_addr=), tmp, type);
+} else {
+ret = virAsprintf(cmd, pci_add %s storage if=%s,
+ (tryOldSyntax ? 0 : pci_addr=auto), type);
+}
+
+if (ret == -1) {
+virReportOOMError(NULL);
+return ret;
+}
+
+if (qemuMonitorCommand(vm, cmd, reply) 0) {
+qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ _(cannot attach %s controller), type);
+VIR_FREE(cmd);
+return -1;
+}
+
+VIR_FREE(cmd);
+
+/* Naturally, the controller hotplug reply is identical with
+ any other PCI hotplug reply */
+if (qemuMonitorParsePciAddReply(vm, reply, domain, bus, slot) 0
[libvirt] [PATCH 10/13] Factor out the method to get the PCI address of a controller for a given disk
From: Wolfgang Mauerer wolfgang.maue...@siemens.com
We need this multiple times later on.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/conf/domain_conf.c | 26 ++
src/conf/domain_conf.h |8 ++
src/qemu/qemu_driver.c | 242 +++-
3 files changed, 153 insertions(+), 123 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e90975f..48015a2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1077,37 +1077,20 @@ virDomainDiskDefParseXML(virConnectPtr conn,
def-unit_id = -1;
if (unit_id virStrToLong_i(unit_id, NULL, 10, def-unit_id) 0) {
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
-_(Cannot parse controller 'unit' attribute));
- goto error;
- }
-
- /* TODO: Should we re-use devaddr for this purpose,
- or is an extra field justified? */
- if (controller_pci_addr
- sscanf(controller_pci_addr, %x:%x:%x,
- def-controller_pci_addr.domain,
- def-controller_pci_addr.bus,
- def-controller_pci_addr.slot) 3) {
- virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
- _(Unable to parse controller 'pci_addr' parameter '%s'),
-controller_pci_addr);
+ _(Cannot parse controller 'unit'
attribute));
goto error;
}
}
/* TODO: Should we re-use devaddr for this purpose,
or is an extra field justified? */
-def-controller_pci_addr.domain = -1;
-def-controller_pci_addr.bus = -1;
-def-controller_pci_addr.slot = -1;
-
if (controller_pci_addr
sscanf(controller_pci_addr, %x:%x:%x,
def-controller_pci_addr.domain,
def-controller_pci_addr.bus,
def-controller_pci_addr.slot) 3) {
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
- _(Unable to parse controller 'pciaddr'
parameter '%s'),
+ _(Unable to parse controller 'pci_addr' parameter '%s'),
controller_pci_addr);
goto error;
}
@@ -1215,6 +1198,11 @@ virDomainControllerDefParseXML(virConnectPtr conn,
}
def-name = virXMLPropString(node, name);
+if (!def-name) {
+virDomainReportError(conn, VIR_ERR_INVALID_ARG,
+ _(cannot use controller without name));
+goto error;
+}
cur = node-children;
while (cur != NULL) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index b30d08c..e058c56 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -171,6 +171,14 @@ virDiskHasValidPciAddr(virDomainDiskDefPtr def)
return def-pci_addr.domain || def-pci_addr.bus || def-pci_addr.slot;
}
+static inline int
+virDiskHasValidController(virDomainDiskDefPtr def)
+{
+return def-controller_name != NULL ||
+def-controller_pci_addr.domain || def-controller_pci_addr.domain
+|| def-controller_pci_addr.slot;
+}
+
/* Two types of disk backends */
enum virDomainFSType {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4f647c4..9f44685 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4356,44 +4356,20 @@ static int
qemudDomainChangeEjectableMedia(virConnectPtr conn,
return ret;
}
-static int qemudDomainAttachDiskDevice(virConnectPtr conn,
- virDomainObjPtr vm,
- virDomainDeviceDefPtr dev)
-{
-int ret, i;
-char *cmd, *reply;
-char *safe_path;
-/* NOTE: Later patch will use type of the controller instead
- of the disk */
-const char* type = virDomainDiskBusTypeToString(dev-data.disk-bus);
-int tryOldSyntax = 0;
-unsigned domain, bus, slot;
-int bus_id, unit_id;
-virBuffer buf = VIR_BUFFER_INITIALIZER;
-char* bus_unit_string;
-int controller_specified;
-
-for (i = 0 ; i vm-def-ndisks ; i++) {
-if (STREQ(vm-def-disks[i]-dst, dev-data.disk-dst)) {
-qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _(target %s already exists), dev-data.disk-dst);
-return -1;
-}
-}
-
-if (VIR_REALLOC_N(vm-def-disks, vm-def-ndisks+1) 0) {
-virReportOOMError(conn);
-return -1;
-}
-
-try_command:
-safe_path = qemudEscapeMonitorArg(dev-data.disk-src);
-if (!safe_path) {
-virReportOOMError(conn);
-return -1;
-}
+enum {
+CONTROLLER_FOUND = 0,
+NO_CONTROLLER_FOUND = -1, /* None specified, and none found */
+CONTROLLER_INEXISTENT = -2, /* Controller specified, but not found */
+};
+static virDomainControllerDefPtr
+qemudGetDiskController(virDomainObjPtr vm
[libvirt] [PATCH 12/13] Extract monitor parts from the qemu interaction for disk hotplug
From: Wolfgang Mauerer wolfgang.maue...@siemens.com
This separates the communication with qemu a bit from the more
libvirtish actions.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/qemu/qemu_driver.c | 82 ++--
src/qemu/qemu_monitor_text.c | 85 ++
src/qemu/qemu_monitor_text.h |5 ++
3 files changed, 95 insertions(+), 77 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 289c3c6..df22035 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4445,15 +4445,8 @@ static int qemudDomainAttachDiskDevice(virConnectPtr
conn,
virDomainDeviceDefPtr dev)
{
int ret, i;
-char *cmd, *reply;
-char *safe_path;
-const char* type = NULL;
-int tryOldSyntax = 0;
-int bus_id, unit_id;
-int domain, bus, slot;
virDomainControllerDefPtr conPtr;
-virBuffer buf = VIR_BUFFER_INITIALIZER;
-char* bus_unit_string;
+const char* type = NULL;
/* Both controller and disk can specify a type like SCSI. While
a virtual disk as such is not bound to a specific bus type,
@@ -4485,13 +4478,6 @@ static int qemudDomainAttachDiskDevice(virConnectPtr
conn,
return -1;
}
-try_command:
-safe_path = qemudEscapeMonitorArg(dev-data.disk-src);
-if (!safe_path) {
-virReportOOMError(conn);
-return -1;
-}
-
conPtr = qemudGetDiskController(vm, dev, ret);
if (!conPtr) {
switch (ret) {
@@ -4510,70 +4496,12 @@ try_command:
/* At this point, we have a valid controller, regardless if an explicit
controller has been specified or not. */
-domain = conPtr-address-data.pci.domain;
-bus= conPtr-address-data.pci.bus;
-slot = conPtr-address-data.pci.slot;
-
-if (dev-data.disk-bus_id != -1) {
- virBufferVSprintf(buf, ,bus=%d, dev-data.disk-bus_id);
-}
-
-if (dev-data.disk-unit_id != -1) {
-virBufferVSprintf(buf, ,unit=%d, dev-data.disk-unit_id);
-}
-
-bus_unit_string = virBufferContentAndReset(buf);
-
-VIR_DEBUG (%s: drive_add %.2x:%.2x:%.2x file=%s,if=%s%s, vm-def-name,
- domain, bus, slot, safe_path, type,
- bus_unit_string ? bus_unit_string : );
-ret = virAsprintf(cmd, drive_add %s%.2x:%.2x:%.2x file=%s,if=%s%s,
- (tryOldSyntax ? : pci_addr=), domain, bus,
- slot, safe_path, type,
- bus_unit_string ? bus_unit_string : );
-
-VIR_FREE(safe_path);
-if (ret == -1) {
-virReportOOMError(conn);
-return ret;
-}
+ret = qemuMonitorAttachDiskDevice(vm, dev, conPtr, type);
-if (qemudMonitorCommand(vm, cmd, reply) 0) {
-qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _(cannot attach %s disk), type);
-VIR_FREE(cmd);
-return -1;
-}
-
-VIR_FREE(cmd);
-
-if (qemudParseDriveAddReply(vm, reply, bus_id, unit_id) 0) {
-if (!tryOldSyntax strstr(reply, invalid char in expression)) {
-VIR_FREE(reply);
-tryOldSyntax = 1;
-goto try_command;
-}
-qemudReportError (conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _(adding %s disk failed: %s), type, reply);
-VIR_FREE(reply);
-return -1;
-}
-
-if (dev-data.disk-bus_id == -1) {
-dev-data.disk-bus_id = bus_id;
-}
-
-if (dev-data.disk-unit_id == -1) {
-dev-data.disk-unit_id = unit_id;
-}
-
-dev-data.disk-pci_addr.domain = domain;
-dev-data.disk-pci_addr.bus= bus;
-dev-data.disk-pci_addr.slot = slot;
-
-virDomainDiskInsertPreAlloced(vm-def, dev-data.disk);
+if (!ret)
+virDomainDiskInsertPreAlloced(vm-def, dev-data.disk);
-return 0;
+return ret;
}
static int qemudDomainAttachDiskController(virConnectPtr conn,
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 7326d05..371388f 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -1594,6 +1594,91 @@ try_command:
return 0;
}
+int qemuMonitorAttachDiskDevice(virDomainObjPtr vm,
+virDomainDeviceDefPtr dev,
+virDomainControllerDefPtr conPtr,
+const char *type)
+{
+int ret;
+char *cmd, *reply;
+char *safe_path;
+int tryOldSyntax = 0;
+int bus_id, unit_id;
+int domain, bus, slot;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+char* bus_unit_string;
+
+domain = conPtr-address-data.pci.domain;
+bus= conPtr-address-data.pci.bus;
+slot = conPtr-address-data.pci.slot;
+
+if (dev-data.disk-bus_id != -1) {
+ virBufferVSprintf(buf, ,bus=%d, dev-data.disk-bus_id);
+}
+
+if (dev
Re: [libvirt] [PATCH 0/9] Support disk-hotremove and controller hotplugging
Hi, Gerd Hoffmann wrote: On 10/09/09 18:32, Daniel Veillard wrote: On Fri, Sep 18, 2009 at 05:26:07PM +0200, Wolfgang Mauerer wrote: Hi, Hi Wolfgang, this patch reworks libvirt's disk-hotadd support and introduces support for disk controller hotplugging and disk-hotremove (see http://thread.gmane.org/gmane.comp.emulators.libvirt/15860 for more details). Currently, it targets only qemu and also requires some additions to qemu that have only recently been submitted, which is why this is cross-posted to qemu-devel. Hopefully the lack of support in qemu does not prevent the community from reviewing the code, though ;-) Could you update us on the status for those patches, is there any chance to get this in QEmu upstream ? I have only seen feddback from Gerd Hoffmann on that thread but maybe I'm missing some of the action ! sorry, I did not get that mail for some reason, so I did not reply. Note: upstream qemu got device_add and device_del monitor commands meanwhile which can be used to hotplug devices. device_add accepts the same syntax the -device command line switch expects. device_del expectes an id. For libvirt it probably makes sense to start supporting this when switching over to -device (probably for qemu 0.12+). Since it does most likely not make sense to include my drive_del implementation into qemu 0.11 mainline, I'll add libvirt support for these after I finish doing the structural changes suggested by Daniel Berrange. Cheers, Wolfgang -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/9] Implement controller hotplugging
Daniel P. Berrange wrote:
On Fri, Sep 18, 2009 at 05:26:12PM +0200, Wolfgang Mauerer wrote:
This enables to hot-add disk controllers without attached
disks into the system. Previously, it was only possible to
(implicitly) add disk controllers in the static machine
configuration.
Notice that the actual functionality is only available
for qemu at present, but other emulators can be extended
likewise.
Any idea what we can do about initial startup? eg, if we start
a guest with 1 SCSI controller and 1 disk, and then we've then
hotplugged a 2nd controller, and 1 disk. When we later boot
or migrate the same guest, we need to have suitable QEMU command
line args to make sure we get 2 controllers each with the same
disk, rather than 1 controller with 2 disks. I'm not sure how
we do this in QEMU, hopefully the existing args support it in
some way I've not realized, or failing that the new qdev -device
args might help us.
I'm not yet sure how to solve that best. I'll take a look at how
the -device can help exactly.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/domain_conf.c| 26 +++---
src/domain_conf.h|2 ++
src/libvirt_private.syms |1 +
src/qemu_driver.c| 21 +
4 files changed, 43 insertions(+), 7 deletions(-)
diff --git a/src/domain_conf.c b/src/domain_conf.c
index d0fda64..ea51fda 100644
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -647,7 +647,6 @@ void virDomainRemoveInactive(virDomainObjListPtr doms,
}
-
/* Parse the XML definition for a disk
* @param node XML nodeset to parse for disk definition
*/
@@ -2554,6 +2553,27 @@ virDomainDeviceDefPtr
virDomainDeviceDefParse(virConnectPtr conn,
#endif
+static int virDomainControllerCompare(virDomainControllerDefPtr a,
+ virDomainControllerDefPtr b) {
+if (a-pci_addr.bus == b-pci_addr.bus) {
+if (a-pci_addr.domain == b-pci_addr.domain)
+return a-pci_addr.slot - b-pci_addr.slot;
+
+return a-pci_addr.domain - b-pci_addr.domain;
+}
+
+return a-pci_addr.bus - b-pci_addr.bus;
+}
+
+
+int virDomainControllerQSort(const void *a, const void *b)
+{
+const virDomainControllerDefPtr *da = a;
+const virDomainControllerDefPtr *db = b;
+
+return virDomainControllerCompare(*da, *db);
+}
+
I know we used todo this for disk devices, but I'd recommand not
going a qsort of devices when hotplugging/unplugging. For hotplug
always append to the list, for unplug just shuffle down later
devices in the list to fill the hole.
okay.
Thanks, Wolfgang
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 9/9] Implement disk- and controller hotremove
Daniel P. Berrange wrote:
On Fri, Sep 18, 2009 at 05:26:16PM +0200, Wolfgang Mauerer wrote:
Since both disks and disk controllers can be dynamically
added to the system, it makes sense to be also able to remove
them.
This is the main patch which requires additional support in QEMu
if I'm understanding your patcheset to qemu-devel correctly.
Yes, that's right.
diff --git a/src/domain_conf.h b/src/domain_conf.h
index 17f8b14..c7d49cf 100644
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -149,6 +149,14 @@ virDiskHasValidPciAddr(virDomainDiskDefPtr def)
return def-pci_addr.domain || def-pci_addr.domain ||
def-pci_addr.slot;
}
+static inline int
+virDiskHasValidController(virDomainDiskDefPtr def)
+{
+return def-controller_id != NULL ||
+def-controller_pci_addr.domain || def-controller_pci_addr.domain
+|| def-controller_pci_addr.slot;
+}
+
/* Two types of disk backends */
enum virDomainFSType {
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index ddc46f6..5ac89a1 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -6204,32 +6204,31 @@ cleanup:
return ret;
}
-static int qemudDomainDetachPciDiskDevice(virConnectPtr conn,
- virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
+static int qemudDomainDetachDiskController(virConnectPtr conn,
+ virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
{
int i, ret = -1;
char *cmd = NULL;
char *reply = NULL;
-virDomainDiskDefPtr detach = NULL;
+virDomainControllerDefPtr detach = NULL;
int tryOldSyntax = 0;
-for (i = 0 ; i vm-def-ndisks ; i++) {
-if (STREQ(vm-def-disks[i]-dst, dev-data.disk-dst)) {
-detach = vm-def-disks[i];
+//virDomainControllerDefPtr is dev-data.controller
+for (i = 0 ; i vm-def-ncontrollers ; i++) {
+if (vm-def-controllers[i]-pci_addr.domain ==
+dev-data.controller-pci_addr.domain
+vm-def-controllers[i]-pci_addr.bus ==
+dev-data.controller-pci_addr.bus
+vm-def-controllers[i]-pci_addr.slot ==
+dev-data.controller-pci_addr.slot) {
+detach = vm-def-controllers[i];
break;
}
}
if (!detach) {
qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
- _(disk %s not found), dev-data.disk-dst);
-goto cleanup;
-}
-
-if (!virDiskHasValidPciAddr(detach)) {
-qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
- _(disk %s cannot be detached - no PCI address for
device),
- detach-dst);
+ _(Controller %s not found), dev-data.disk-dst);
goto cleanup;
}
@@ -6251,7 +6250,9 @@ try_command:
if (qemudMonitorCommand(vm, cmd, reply) 0) {
qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
- _(failed to execute detach disk %s command),
detach-dst);
+ _(failed to execute detach controller %d:%d:%d \
+ command), detach-pci_addr.domain,
+ detach-pci_addr.bus, detach-pci_addr.slot);
goto cleanup;
}
@@ -6267,6 +6268,124 @@ try_command:
if (strstr(reply, invalid slot) ||
strstr(reply, Invalid pci address)) {
qemudReportError (conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ _(failed to detach controller: invalid PCI
address %.4x:%.2x:%.2x: %s),
+ detach-pci_addr.domain,
+ detach-pci_addr.bus,
+ detach-pci_addr.slot,
+ reply);
+goto cleanup;
+}
+
+if (vm-def-ncontrollers 1) {
+vm-def-controllers[i] =
vm-def-controllers[--vm-def-ncontrollers];
+if (VIR_REALLOC_N(vm-def-controllers, vm-def-ncontrollers) 0)
{
+virReportOOMError(conn);
+goto cleanup;
+}
+qsort(vm-def-disks, vm-def-ncontrollers,
+ sizeof(*vm-def-controllers),
+ virDomainControllerQSort);
+} else {
+VIR_FREE(vm-def-controllers[0]);
+vm-def-controllers = 0;
+}
+ret = 0;
+
+cleanup:
+VIR_FREE(reply);
+VIR_FREE(cmd);
+return ret;
+}
+
+static int qemudDomainDetachDiskDevice(virConnectPtr conn,
+ virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
+{
+int i, ret = -1;
+char *cmd = NULL;
+char *reply = NULL;
+const char* type;
+virDomainDiskDefPtr detach = NULL;
+int tryOldSyntax = 0;
+
+/* If bus and unit are specified, use these first to identify
+ the disk */
+if (dev-data.disk-controller_pci_addr.domain != -1
Re: [libvirt] [PATCH 3/9] Add new domain device: controller
Daniel P. Berrange wrote:
On Fri, Sep 18, 2009 at 05:26:10PM +0200, Wolfgang Mauerer wrote:
This augments virDomainDevice with a controller element
that is used to represent disk controllers (e.g., scsi
controllers). The XML format is given by
controller type=scsi id=my_id
bus addr=Domain:Bus:Slot
/controller
where type denotes the disk interface (scsi, ide,...), id
is an arbitrary string that identifies the controller for
disk hotadd/remove, and bus addr denotes the controller address
on the PCI bus.
The bus element can be omitted; in this case,
an address will be assigned automatically. Currently,
only hotplugging scsi controllers on a PCI bus
is supported, and this only for qemu guests
As mentioned in the previous patch, I reckon 'id' is better
called 'name'.
For PCI addresses, it is desirable to fully normalize the XML
format, by which I mean have separate attributes for domain,
bus and slot. We already have a syntax for PCI addresses used
for host device passthrough, so it'd make sense to use the
same syntax here for controllers. More broadly, we're probably
going to have to add a PCI address element to all our devices.
While it is unlikely we'll need non-PCI addresses, it doesn't
hurt to make it explicit by adding a type='pci' attribute
Thus I'd suggest
address type='pci' domain='0x' bus='0x06' slot='0x12'/
Instead of
bus addr=Domain:Bus:Slot
In the domain_conf.c/.h parser, we could have a datatype like
enum {
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST
};
typedef struct _virDomainDevicePCIAddress virDomainDevicePCIAddress;
struct _virDomainDevicePCIAddress {
unsigned domain;
unsigned bus;
unsigned slot;
unsigned function;
};
typedef struct _virDomainDeviceAddress virDomainDeviceAddress;
struct _virDomainDeviceAddress {
int type;
union {
virDomainDevicePCIAddress pci;
} data;
};
Which we then use in all the places in domain_conf.h wanting
address information. Obviously we'd not use the 'function'
field in most places, but doesn't hurt to have it.
And a pair of methods for parsing/formatting this address info
we can call from all the appropriate locations.
using a generic format for PCi addresses surely makes sense,
especially wrt. the common data structure and parsing routines -
I'll add these into the next series. However, I'm not sure
if using address type='pci' domain='0x' bus='0x06' slot='0x12'/
instead of a simple string provides much of an advantage
and does not just increase the typing strain (though there are
maybe some small advantages for computerised parsing of libvirt
configuration files by other apps).
There's already a generic definition of PCI addresses in
docs/schemas/domain.rng:\approx 1140, I suppose it's your
intention to extend if by a type field?
Thanks, Wolfgang
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/9] Extend disk element with controller information
Matthias Bolte wrote: 2009/9/24 Daniel P. Berrange berra...@redhat.com: Outside the scope of your patches, I think it would be worth adding a 'name' attribute to all devices in the libvirt XML as a standardized unique identifier. We already have to keep track of a 'name' internally for NIC hotplug with QEMU, and with QEMU's qdev work we're going to end up having to track a 'name' for pretty much all devices. Thus we might as well expose it in the XML Would this device name be read-only, or would it be editable/writable by the user (e.g. through virsh edit)? If it should be read-only, I see no problem supporting device names in the ESX driver. But If it should be editable/writable it may be a problem to implement this for the ESX driver, as I'm currently not aware of any sensible way to store such an custom device name in a persistent manner on an ESX server. I suppose having the name read-only suffices. At least I don't see any particularly important use cases for a writeable name, especially considering that the name can be chosen arbitrarily at creation time. And there's still the possibility to shut down the virtual machine, change names, and restart again if new names are absolutely required. Thanks, Wolfgang -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/9] Support disk-hotremove and controller hotplugging
Hi, this patch reworks libvirt's disk-hotadd support and introduces support for disk controller hotplugging and disk-hotremove (see http://thread.gmane.org/gmane.comp.emulators.libvirt/15860 for more details). Currently, it targets only qemu and also requires some additions to qemu that have only recently been submitted, which is why this is cross-posted to qemu-devel. Hopefully the lack of support in qemu does not prevent the community from reviewing the code, though ;-) Notice that no documentation and testcases are included yet; I'll supply those once the interface is fixed and possible issues are settled. Thanks for your comments, Wolfgang docs/schemas/domain.rng | 145 ++ src/domain_conf.c| 208 - src/domain_conf.h| 39 - src/libvirt_private.syms |4 +- src/qemu_driver.c| 479 +++--- 5 files changed, 802 insertions(+), 73 deletions(-) -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/9] Remove surprises in the semantics of disk-hotadd
When a disk is added without an explicitly specified
controller as host, then try to find the first available
controller. If none exists, do not (as in previous versions)
add a new PCI controller device with the disk attached,
but bail out with an error. Notice that this patch changes
the behaviour as compared to older libvirt releases, as
has been discussed on the mailing list (see
http://thread.gmane.org/gmane.comp.emulators.libvirt/15860)
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/qemu_driver.c | 172 ++---
1 files changed, 85 insertions(+), 87 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 990f05a..f1c2a45 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -5417,68 +5417,81 @@ try_command:
controller_specified = 1;
}
-if (controller_specified) {
-if (dev-data.disk-controller_id) {
-for (i = 0 ; i vm-def-ncontrollers ; i++) {
-if (STREQ(dev-data.disk-controller_id,
- vm-def-controllers[i]-id)) {
-break;
-}
-}
-
-if (i == vm-def-ncontrollers) {
-qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _(Controller does not exist));
-return -1;
-}
+if (!controller_specified) {
+/* Find an appropriate controller for disk-hotadd. Notice that
+ the admissible controller types (SCSI, virtio) have already
+ been checked in qemudDomainAttachDevice. */
+for (i = 0 ; i vm-def-ncontrollers ; i++) {
+if (vm-def-controllers[i]-type == dev-data.disk-type)
+break;
+}
+
+if (i == vm-def-ncontrollers) {
+qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ _(Cannot find appropriate controller for hot-add));
+return -1;
+}
-domain = vm-def-controllers[i]-pci_addr.domain;
-bus= vm-def-controllers[i]-pci_addr.bus;
-slot = vm-def-controllers[i]-pci_addr.slot;
-} else {
-domain = dev-data.disk-controller_pci_addr.domain;
-bus= dev-data.disk-controller_pci_addr.bus;
-slot = dev-data.disk-controller_pci_addr.slot;
-
-for (i = 0 ; i vm-def-ncontrollers ; i++) {
-if (domain == vm-def-controllers[i]-pci_addr.domain
-bus== vm-def-controllers[i]-pci_addr.bus
-slot == vm-def-controllers[i]-pci_addr.slot)
-break;
-}
+domain = vm-def-controllers[i]-pci_addr.domain;
+bus= vm-def-controllers[i]-pci_addr.bus;
+slot = vm-def-controllers[i]-pci_addr.slot;
+}
-if (i == vm-def-ncontrollers) {
-qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _(Controller does not exist));
-return -1;
+if (controller_specified dev-data.disk-controller_id) {
+for (i = 0 ; i vm-def-ncontrollers ; i++) {
+if (STREQ(dev-data.disk-controller_id,
+ vm-def-controllers[i]-id)) {
+break;
}
- }
-
-if (dev-data.disk-bus_id != -1) {
-virBufferVSprintf(buf, ,bus=%d, dev-data.disk-bus_id);
}
-if (dev-data.disk-unit_id != -1) {
-virBufferVSprintf(buf, ,unit=%d, dev-data.disk-unit_id);
+if (i == vm-def-ncontrollers) {
+qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ _(Controller does not exist));
+return -1;
+}
+
+domain = vm-def-controllers[i]-pci_addr.domain;
+bus= vm-def-controllers[i]-pci_addr.bus;
+slot = vm-def-controllers[i]-pci_addr.slot;
+} else if (controller_specified) {
+domain = dev-data.disk-controller_pci_addr.domain;
+bus= dev-data.disk-controller_pci_addr.bus;
+slot = dev-data.disk-controller_pci_addr.slot;
+
+for (i = 0 ; i vm-def-ncontrollers ; i++) {
+if (domain == vm-def-controllers[i]-pci_addr.domain
+bus== vm-def-controllers[i]-pci_addr.bus
+slot == vm-def-controllers[i]-pci_addr.slot)
+break;
}
+
+if (i == vm-def-ncontrollers) {
+qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ _(Controller does not exist));
+return -1;
+}
+}
-bus_unit_string = virBufferContentAndReset(buf);
-
-VIR_DEBUG (%s: drive_add %d:%d:%d file=%s,if=%s%s, vm-def-name,
- domain, bus, slot, safe_path, type
[libvirt] [PATCH 9/9] Implement disk- and controller hotremove
Since both disks and disk controllers can be dynamically
added to the system, it makes sense to be also able to remove
them.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/domain_conf.h |8 +++
src/qemu_driver.c | 161 +++-
2 files changed, 153 insertions(+), 16 deletions(-)
diff --git a/src/domain_conf.h b/src/domain_conf.h
index 17f8b14..c7d49cf 100644
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -149,6 +149,14 @@ virDiskHasValidPciAddr(virDomainDiskDefPtr def)
return def-pci_addr.domain || def-pci_addr.domain || def-pci_addr.slot;
}
+static inline int
+virDiskHasValidController(virDomainDiskDefPtr def)
+{
+return def-controller_id != NULL ||
+def-controller_pci_addr.domain || def-controller_pci_addr.domain
+|| def-controller_pci_addr.slot;
+}
+
/* Two types of disk backends */
enum virDomainFSType {
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index ddc46f6..5ac89a1 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -6204,32 +6204,31 @@ cleanup:
return ret;
}
-static int qemudDomainDetachPciDiskDevice(virConnectPtr conn,
- virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
+static int qemudDomainDetachDiskController(virConnectPtr conn,
+ virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
{
int i, ret = -1;
char *cmd = NULL;
char *reply = NULL;
-virDomainDiskDefPtr detach = NULL;
+virDomainControllerDefPtr detach = NULL;
int tryOldSyntax = 0;
-for (i = 0 ; i vm-def-ndisks ; i++) {
-if (STREQ(vm-def-disks[i]-dst, dev-data.disk-dst)) {
-detach = vm-def-disks[i];
+//virDomainControllerDefPtr is dev-data.controller
+for (i = 0 ; i vm-def-ncontrollers ; i++) {
+if (vm-def-controllers[i]-pci_addr.domain ==
+dev-data.controller-pci_addr.domain
+vm-def-controllers[i]-pci_addr.bus ==
+dev-data.controller-pci_addr.bus
+vm-def-controllers[i]-pci_addr.slot ==
+dev-data.controller-pci_addr.slot) {
+detach = vm-def-controllers[i];
break;
}
}
if (!detach) {
qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
- _(disk %s not found), dev-data.disk-dst);
-goto cleanup;
-}
-
-if (!virDiskHasValidPciAddr(detach)) {
-qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
- _(disk %s cannot be detached - no PCI address for
device),
- detach-dst);
+ _(Controller %s not found), dev-data.disk-dst);
goto cleanup;
}
@@ -6251,7 +6250,9 @@ try_command:
if (qemudMonitorCommand(vm, cmd, reply) 0) {
qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
- _(failed to execute detach disk %s command),
detach-dst);
+ _(failed to execute detach controller %d:%d:%d \
+ command), detach-pci_addr.domain,
+ detach-pci_addr.bus, detach-pci_addr.slot);
goto cleanup;
}
@@ -6267,6 +6268,124 @@ try_command:
if (strstr(reply, invalid slot) ||
strstr(reply, Invalid pci address)) {
qemudReportError (conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ _(failed to detach controller: invalid PCI address
%.4x:%.2x:%.2x: %s),
+ detach-pci_addr.domain,
+ detach-pci_addr.bus,
+ detach-pci_addr.slot,
+ reply);
+goto cleanup;
+}
+
+if (vm-def-ncontrollers 1) {
+vm-def-controllers[i] =
vm-def-controllers[--vm-def-ncontrollers];
+if (VIR_REALLOC_N(vm-def-controllers, vm-def-ncontrollers) 0) {
+virReportOOMError(conn);
+goto cleanup;
+}
+qsort(vm-def-disks, vm-def-ncontrollers,
+ sizeof(*vm-def-controllers),
+ virDomainControllerQSort);
+} else {
+VIR_FREE(vm-def-controllers[0]);
+vm-def-controllers = 0;
+}
+ret = 0;
+
+cleanup:
+VIR_FREE(reply);
+VIR_FREE(cmd);
+return ret;
+}
+
+static int qemudDomainDetachDiskDevice(virConnectPtr conn,
+ virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
+{
+int i, ret = -1;
+char *cmd = NULL;
+char *reply = NULL;
+const char* type;
+virDomainDiskDefPtr detach = NULL;
+int tryOldSyntax = 0;
+
+/* If bus and unit are specified, use these first to identify
+ the disk */
+if (dev-data.disk-controller_pci_addr.domain != -1) {
+for (i = 0 ; i vm-def-ndisks ; i++) {
+if (dev-data.disk-bus_id != -1
+dev
[libvirt] [PATCH 2/9] Extend disk element with controller information
This allows us to connect a disk with a specific controller,
which is required for disk hotadd/remove. A new XML child
element is added to the disk container:
disk
...
controller pci_addr=addr id=identifier bus=number
unit=number/
/disk
Either id _or_ pci_addr can be specified. When the controller
has been brought into the system via tghe hotplug mechanism also
included in this patch series, it will typically have an ID.
In other cases, the controller can also be specified with
the PCI address.
Wrt. the data structures touched in this commit, notice
that while bus as subelement of target specifies
the type of bus (e.g., scsi, ide,...), it is used
to enumerate the buses on the controller here.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/domain_conf.c | 29 +
src/domain_conf.h |5 -
2 files changed, 33 insertions(+), 1 deletions(-)
diff --git a/src/domain_conf.c b/src/domain_conf.c
index 5ae0775..a6120c8 100644
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -650,7 +650,12 @@ virDomainDiskDefParseXML(virConnectPtr conn,
char *driverType = NULL;
char *source = NULL;
char *target = NULL;
+char *controller = NULL;
+char *bus_id = NULL;
+char *unit_id = NULL;
+char *controller_id = NULL;
char *bus = NULL;
+char *unit = NULL;
char *cachetag = NULL;
char *devaddr = NULL;
virStorageEncryptionPtr encryption = NULL;
@@ -694,12 +699,18 @@ virDomainDiskDefParseXML(virConnectPtr conn,
(xmlStrEqual(cur-name, BAD_CAST target))) {
target = virXMLPropString(cur, dev);
bus = virXMLPropString(cur, bus);
+unit = virXMLPropString(cur, unit);
/* HACK: Work around for compat with Xen
* driver in previous libvirt releases */
if (target
STRPREFIX(target, ioemu:))
memmove(target, target+6, strlen(target)-5);
+} else if ((controller == NULL)
+ (xmlStrEqual(cur-name, BAD_CAST controller))) {
+ controller_id = virXMLPropString(cur, id);
+ bus_id = virXMLPropString(cur, bus);
+ unit_id = virXMLPropString(cur, unit);
} else if ((driverName == NULL)
(xmlStrEqual(cur-name, BAD_CAST driver))) {
driverName = virXMLPropString(cur, name);
@@ -800,6 +811,24 @@ virDomainDiskDefParseXML(virConnectPtr conn,
}
}
+if (controller) {
+def-controller_id = controller_id;
+
+ def-bus_id = -1;
+ if (bus_id virStrToLong_i(bus_id, NULL, 10, def-bus_id) 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+_(Cannot parse controller 'bus' attribute));
+ goto error;
+ }
+
+ def-unit_id = -1;
+ if (unit_id virStrToLong_i(unit_id, NULL, 10, def-unit_id) 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+_(Cannot parse controller 'unit' attribute));
+ goto error;
+ }
+}
+
if (def-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY
def-bus != VIR_DOMAIN_DISK_BUS_FDC) {
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
diff --git a/src/domain_conf.h b/src/domain_conf.h
index 09368d9..898f6c9 100644
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -105,9 +105,12 @@ typedef virDomainDiskDef *virDomainDiskDefPtr;
struct _virDomainDiskDef {
int type;
int device;
-int bus;
+int bus; /* Bus type, e.g. scsi or ide */
+int bus_id; /* Bus number on the controller */
+int unit_id; /* Unit on the controller */
char *src;
char *dst;
+char *controller_id;
char *driverName;
char *driverType;
char *serial;
--
1.6.4
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/9] Implement controller hotplugging
This enables to hot-add disk controllers without attached
disks into the system. Previously, it was only possible to
(implicitly) add disk controllers in the static machine
configuration.
Notice that the actual functionality is only available
for qemu at present, but other emulators can be extended
likewise.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/domain_conf.c| 26 +++---
src/domain_conf.h|2 ++
src/libvirt_private.syms |1 +
src/qemu_driver.c| 21 +
4 files changed, 43 insertions(+), 7 deletions(-)
diff --git a/src/domain_conf.c b/src/domain_conf.c
index d0fda64..ea51fda 100644
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -647,7 +647,6 @@ void virDomainRemoveInactive(virDomainObjListPtr doms,
}
-
/* Parse the XML definition for a disk
* @param node XML nodeset to parse for disk definition
*/
@@ -2554,6 +2553,27 @@ virDomainDeviceDefPtr
virDomainDeviceDefParse(virConnectPtr conn,
#endif
+static int virDomainControllerCompare(virDomainControllerDefPtr a,
+ virDomainControllerDefPtr b) {
+if (a-pci_addr.bus == b-pci_addr.bus) {
+if (a-pci_addr.domain == b-pci_addr.domain)
+return a-pci_addr.slot - b-pci_addr.slot;
+
+return a-pci_addr.domain - b-pci_addr.domain;
+}
+
+return a-pci_addr.bus - b-pci_addr.bus;
+}
+
+
+int virDomainControllerQSort(const void *a, const void *b)
+{
+const virDomainControllerDefPtr *da = a;
+const virDomainControllerDefPtr *db = b;
+
+return virDomainControllerCompare(*da, *db);
+}
+
int virDomainDiskInsert(virDomainDefPtr def,
virDomainDiskDefPtr disk)
{
@@ -2935,8 +2955,8 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr
conn,
def-controllers[def-ncontrollers++] = controller;
}
-/*qsort(def-controllers, def-ncontrollers, sizeof(*def-controllers),
- virDomainControllerQSort); */
+qsort(def-controllers, def-ncontrollers, sizeof(*def-controllers),
+ virDomainControllerQSort);
VIR_FREE(nodes);
diff --git a/src/domain_conf.h b/src/domain_conf.h
index 41df8f6..17f8b14 100644
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -728,6 +728,8 @@ int virDomainDiskInsert(virDomainDefPtr def,
void virDomainDiskInsertPreAlloced(virDomainDefPtr def,
virDomainDiskDefPtr disk);
+int virDomainControllerQSort(const void *a, const void *b);
+
int virDomainSaveXML(virConnectPtr conn,
const char *configDir,
virDomainDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f724493..ecc1123 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -86,6 +86,7 @@ virDomainDiskDefFree;
virDomainDiskDeviceTypeToString;
virDomainDiskInsert;
virDomainDiskInsertPreAlloced;
+virDomainControllerQSort;
virDomainFindByID;
virDomainFindByName;
virDomainFindByUUID;
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 4a30615..3bdd2d7 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -5424,6 +5424,15 @@ try_command:
bus= dev-data.disk-controller_pci_addr.bus;
slot = dev-data.disk-controller_pci_addr.slot;
+if (dev-data.disk-controller_id) {
+/* TODO: Obtain the PCI address of the controller
+ from the data structures using the ID */
+} else {
+domain = dev-data.disk-controller_pci_addr.domain;
+bus= dev-data.disk-controller_pci_addr.bus;
+slot = dev-data.disk-controller_pci_addr.slot;
+ }
+
if (dev-data.disk-bus_id != -1) {
virBufferVSprintf(buf, ,bus=%d, dev-data.disk-bus_id);
}
@@ -5582,6 +5591,8 @@ try_command:
VIR_FREE(cmd);
+/* Naturally, the controller hotplug reply is identical with
+ any other PCI hotplug reply */
if (qemudParsePciAddReply(vm, reply, domain, bus, slot) 0) {
if (!tryOldSyntax strstr(reply, invalid char in expression)) {
VIR_FREE(reply);
@@ -5596,17 +5607,17 @@ try_command:
}
/* Also fill in when the address was explicitely specified in
- case qemu changed it (TODO: Can this really happen?) */
+ case qemu changed it */
dev-data.controller-pci_addr.domain = domain;
dev-data.controller-pci_addr.bus= bus;
dev-data.controller-pci_addr.slot = slot;
VIR_FREE(reply);
-/* NOTE: Sort function is added in a later commit */
vm-def-controllers[vm-def-ncontrollers++] = dev-data.controller;
-/*qsort(vm-def-disks, vm-def-ndisks, sizeof(*vm-def-disks),
-virDomainDiskQSort);*/
+qsort(vm-def-controllers, vm-def-ncontrollers,
+ sizeof(*vm-def-controllers),
+ virDomainControllerQSort);
return 0;
}
@@ -6104,6 +6115,8
[libvirt] [PATCH 3/9] Add new domain device: controller
This augments virDomainDevice with a controller element that is used to represent disk controllers (e.g., scsi controllers). The XML format is given by controller type=scsi id=my_id bus addr=Domain:Bus:Slot /controller where type denotes the disk interface (scsi, ide,...), id is an arbitrary string that identifies the controller for disk hotadd/remove, and bus addr denotes the controller address on the PCI bus. The bus element can be omitted; in this case, an address will be assigned automatically. Currently, only hotplugging scsi controllers on a PCI bus is supported, and this only for qemu guests Routines for parsing this definition and the associated data structures are included in this commit. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- docs/schemas/domain.rng | 145 ++- src/domain_conf.c | 125 src/domain_conf.h | 24 3 files changed, 255 insertions(+), 39 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 70e98a7..dc9b849 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -364,49 +364,116 @@ define name=disk element name=disk optional -attribute name=device + attribute name=device + choice + valuefloppy/value + valuedisk/value + valuecdrom/value + /choice + /attribute + /optional + optional + element name=controller + optional + attribute name=bus + ref name=unsignedInt/ + /attribute + /optional + optional + attribute name=unit + ref name=unsignedInt/ + /attribute + /optional + choice + attribute name=id + ref name=genericName/ + /attribute + attribute name=pciaddr + !-- Just for testing -- + ref name=deviceName/ + /attribute + /choice + /element + /optional + choice + group + attribute name=type + valuefile/value + /attribute + interleave + optional + element name=source + attribute name=file + ref name=absFilePath/ + /attribute + empty/ + /element + /optional + ref name=diskspec/ + /interleave + /group + group + attribute name=type + valueblock/value + /attribute + interleave + optional + element name=source + attribute name=dev + ref name=deviceName/ + /attribute + empty/ + /element + /optional + ref name=diskspec/ + /interleave + /group + ref name=diskspec/ + /choice +/element + /define + define name=target +element name=target + attribute name=dev +ref name=deviceName/ + /attribute + optional +attribute name=bus choice -valuefloppy/value -valuedisk/value -valuecdrom/value +valueide/value +valuefdc/value +valuescsi/value +valuevirtio/value +valuexen/value +valueusb/value +valueuml/value /choice /attribute /optional - choice -group - attribute name=type -valuefile/value - /attribute - interleave -optional - element name=source -attribute name=file - ref name=absFilePath/ -/attribute -empty/ - /element -/optional -ref name=diskspec/ - /interleave -/group -group - attribute name=type -valueblock/value - /attribute - interleave -optional - element name=source -attribute name=dev - ref name=deviceName/ -/attribute -empty/ - /element -/optional -ref name=diskspec/ - /interleave -/group -ref name=diskspec/ - /choice +/element + /define + + define name=controller +element name=controller + interleave + attribute name=type + choice + !-- For now, only SCSI is supported -- + valuescsi/value + /choice + /attribute + attribute name=id + ref name=genericName/ + /attribute + /interleave + optional + element name=bus + attribute name=addr + !-- Just for testing, should be pciaddress -- + ref name=deviceName/ + /attribute
[libvirt] [PATCH 6/9] Allow controller selection by ID
... by simply traversing the list of controllers
to find the associated PCI address.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/qemu_driver.c | 41 +++--
1 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 3bdd2d7..990f05a 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -5418,19 +5418,40 @@ try_command:
}
if (controller_specified) {
-/* NOTE: Proper check for the controller will be implemented
- in a later commit */
- domain = dev-data.disk-controller_pci_addr.domain;
- bus= dev-data.disk-controller_pci_addr.bus;
- slot = dev-data.disk-controller_pci_addr.slot;
-
if (dev-data.disk-controller_id) {
-/* TODO: Obtain the PCI address of the controller
- from the data structures using the ID */
+for (i = 0 ; i vm-def-ncontrollers ; i++) {
+if (STREQ(dev-data.disk-controller_id,
+ vm-def-controllers[i]-id)) {
+break;
+}
+}
+
+if (i == vm-def-ncontrollers) {
+qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ _(Controller does not exist));
+return -1;
+}
+
+domain = vm-def-controllers[i]-pci_addr.domain;
+bus= vm-def-controllers[i]-pci_addr.bus;
+slot = vm-def-controllers[i]-pci_addr.slot;
} else {
domain = dev-data.disk-controller_pci_addr.domain;
bus= dev-data.disk-controller_pci_addr.bus;
slot = dev-data.disk-controller_pci_addr.slot;
+
+for (i = 0 ; i vm-def-ncontrollers ; i++) {
+if (domain == vm-def-controllers[i]-pci_addr.domain
+bus== vm-def-controllers[i]-pci_addr.bus
+slot == vm-def-controllers[i]-pci_addr.slot)
+break;
+}
+
+if (i == vm-def-ncontrollers) {
+qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ _(Controller does not exist));
+return -1;
+}
}
if (dev-data.disk-bus_id != -1) {
@@ -5457,13 +5478,13 @@ try_command:
ret = virAsprintf(cmd, pci_add %s storage file=%s,if=%s,
(tryOldSyntax ? 0: pci_addr=auto), safe_path, type);
}
-
+
VIR_FREE(safe_path);
if (ret == -1) {
virReportOOMError(conn);
return ret;
}
-
+
if (qemudMonitorCommand(vm, cmd, reply) 0) {
qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
_(cannot attach %s disk), type);
--
1.6.4
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/9] Add disk-based hotplugging for the qemu backend
When disks are added to a qemu backend with attach-device,
not just the disk, but a complete new PCI controller with
the disk attached is brought into the system. This patch
implements a proper disk hotplugging scheme for qemu.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/domain_conf.c | 46 --
src/domain_conf.h |2 +-
src/qemu_driver.c | 257
3 files changed, 277 insertions(+), 28 deletions(-)
diff --git a/src/domain_conf.c b/src/domain_conf.c
index bbaf944..d0fda64 100644
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -663,7 +663,6 @@ virDomainDiskDefParseXML(virConnectPtr conn,
char *driverType = NULL;
char *source = NULL;
char *target = NULL;
-char *controller = NULL;
char *controller_pci_addr = NULL;
char *bus_id = NULL;
char *unit_id = NULL;
@@ -720,12 +719,13 @@ virDomainDiskDefParseXML(virConnectPtr conn,
if (target
STRPREFIX(target, ioemu:))
memmove(target, target+6, strlen(target)-5);
-} else if ((controller == NULL)
+} else if ((controller_id == NULL
+controller_pci_addr == NULL)
(xmlStrEqual(cur-name, BAD_CAST controller))) {
- controller_id = virXMLPropString(cur, id);
- controller_pci_addr = virXMLPropString(cur, pci_addr);
- bus_id = virXMLPropString(cur, bus);
- unit_id = virXMLPropString(cur, unit);
+controller_id = virXMLPropString(cur, id);
+controller_pci_addr = virXMLPropString(cur, pciaddr);
+bus_id = virXMLPropString(cur, bus);
+unit_id = virXMLPropString(cur, unit);
} else if ((driverName == NULL)
(xmlStrEqual(cur-name, BAD_CAST driver))) {
driverName = virXMLPropString(cur, name);
@@ -826,7 +826,13 @@ virDomainDiskDefParseXML(virConnectPtr conn,
}
}
-if (controller) {
+if (controller_id controller_pci_addr) {
+virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+ _(Both controller ID and address specified!));
+goto error;
+}
+
+if (controller_id) {
def-controller_id = controller_id;
def-bus_id = -1;
@@ -857,12 +863,35 @@ virDomainDiskDefParseXML(virConnectPtr conn,
}
}
+/* TODO: Should we re-use devaddr for this purpose,
+ or is an extra field justified? */
+def-controller_pci_addr.domain = -1;
+def-controller_pci_addr.bus = -1;
+def-controller_pci_addr.slot = -1;
+
+if (controller_pci_addr
+sscanf(controller_pci_addr, %x:%x:%x,
+ def-controller_pci_addr.domain,
+ def-controller_pci_addr.bus,
+ def-controller_pci_addr.slot) 3) {
+virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(Unable to parse controller 'pciaddr'
parameter '%s'),
+ controller_pci_addr);
+goto error;
+}
+
+VIR_DEBUG(Controller PCI addr for disk parsed as %d:%d:%d\n,
+ def-controller_pci_addr.domain,
+ def-controller_pci_addr.bus,
+ def-controller_pci_addr.slot);
+
if (def-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY
def-bus != VIR_DOMAIN_DISK_BUS_FDC) {
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
_(Invalid bus type '%s' for floppy disk), bus);
goto error;
}
+
if (def-device != VIR_DOMAIN_DISK_DEVICE_FLOPPY
def-bus == VIR_DOMAIN_DISK_BUS_FDC) {
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
@@ -888,6 +917,9 @@ virDomainDiskDefParseXML(virConnectPtr conn,
goto error;
}
+VIR_DEBUG(Disk PCI address parsed as %d:%d:%d\n,
+ def-pci_addr.domain, def-pci_addr.bus, def-pci_addr.slot);
+
def-src = source;
source = NULL;
def-dst = target;
diff --git a/src/domain_conf.h b/src/domain_conf.h
index 6b3cb09..41df8f6 100644
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -107,7 +107,7 @@ struct _virDomainDiskDef {
int device;
int bus; /* Bus type, e.g. scsi or ide */
int bus_id; /* Bus number on the controller */
-int unit_id; /* Unit on the controller */
+int unit_id; /* Unit on the controller (both -1 if unspecified) */
char *src;
char *dst;
char *controller_id;
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index a65334f..4a30615 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -5264,7 +5264,7 @@ qemudParsePciAddReply(virDomainObjPtr vm,
{
char *s, *e;
-DEBUG(%s: pci_add reply: %s, vm-def-name, reply);
+VIR_DEBUG(%s: pci_add reply: %s, vm-def-name, reply);
/* If the command succeeds qemu
[libvirt] [PATCH 1/9] Clarify documentation for private symbols
The instruction See Makefile.am in libvirt.private_syms always makes me think that this file is autogenerated and should not be touched manually. This patch spares every reader of libvirt.private_syms the hassle of reading Makefile.am before augmenting libvirt.private_syms. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- src/libvirt_private.syms |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 867678f..f724493 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1,5 +1,6 @@ # -# General private symbols. See Makefile.am. +# General private symbols. Add symbols here, and see Makefile.am for +# more details. # -- 1.6.4 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 8/9] Factor out the method to get the PCI address of a controller for a given disk
We need this multiple times later on.
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
src/qemu_driver.c | 155 +
1 files changed, 97 insertions(+), 58 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index f1c2a45..ddc46f6 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -5370,44 +5370,20 @@ qemudParseDriveAddReply(virDomainObjPtr vm,
return 0;
}
-static int qemudDomainAttachDiskDevice(virConnectPtr conn,
- virDomainObjPtr vm,
- virDomainDeviceDefPtr dev)
-{
-int ret, i;
-char *cmd, *reply;
-char *safe_path;
-/* NOTE: Later patch will use type of the controller instead
- of the disk */
-const char* type = virDomainDiskBusTypeToString(dev-data.disk-bus);
-int tryOldSyntax = 0;
-unsigned domain, bus, slot;
-int bus_id, unit_id;
-virBuffer buf = VIR_BUFFER_INITIALIZER;
-char* bus_unit_string;
-int controller_specified;
-
-for (i = 0 ; i vm-def-ndisks ; i++) {
-if (STREQ(vm-def-disks[i]-dst, dev-data.disk-dst)) {
-qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _(target %s already exists), dev-data.disk-dst);
-return -1;
-}
-}
-
-if (VIR_REALLOC_N(vm-def-disks, vm-def-ndisks+1) 0) {
-virReportOOMError(conn);
-return -1;
-}
-
-try_command:
-safe_path = qemudEscapeMonitorArg(dev-data.disk-src);
-if (!safe_path) {
-virReportOOMError(conn);
-return -1;
-}
+enum {
+CONTROLLER_FOUND = 0,
+NO_CONTROLLER_FOUND = -1, /* None specified, and none found */
+CONTROLLER_INEXISTENT = -2, /* Controller specified, but not found */
+};
+static virDomainControllerDefPtr
+qemudGetDiskController(virDomainObjPtr vm, virDomainDeviceDefPtr dev,
+ int* ret) {
+int controller_specified = 0;
+int domain, bus, slot;
+virDomainControllerDefPtr conPtr = NULL;
+int i;
+*ret = CONTROLLER_FOUND;
-controller_specified = 0;
/* Partially specified PCI addresses (should not happen, anyway)
don't count as specification */
if (dev-data.disk-controller_id != NULL ||
@@ -5422,38 +5398,28 @@ try_command:
the admissible controller types (SCSI, virtio) have already
been checked in qemudDomainAttachDevice. */
for (i = 0 ; i vm-def-ncontrollers ; i++) {
-if (vm-def-controllers[i]-type == dev-data.disk-type)
+if (vm-def-controllers[i]-type == dev-data.disk-type)
+conPtr = vm-def-controllers[i];
break;
}
-if (i == vm-def-ncontrollers) {
-qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _(Cannot find appropriate controller for hot-add));
-return -1;
+if (!conPtr) {
+*ret = NO_CONTROLLER_FOUND;
}
-
-domain = vm-def-controllers[i]-pci_addr.domain;
-bus= vm-def-controllers[i]-pci_addr.bus;
-slot = vm-def-controllers[i]-pci_addr.slot;
}
if (controller_specified dev-data.disk-controller_id) {
for (i = 0 ; i vm-def-ncontrollers ; i++) {
if (STREQ(dev-data.disk-controller_id,
vm-def-controllers[i]-id)) {
+conPtr = vm-def-controllers[i];
break;
}
}
-if (i == vm-def-ncontrollers) {
-qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _(Controller does not exist));
-return -1;
+if (!conPtr) {
+*ret = CONTROLLER_INEXISTENT;
}
-
-domain = vm-def-controllers[i]-pci_addr.domain;
-bus= vm-def-controllers[i]-pci_addr.bus;
-slot = vm-def-controllers[i]-pci_addr.slot;
} else if (controller_specified) {
domain = dev-data.disk-controller_pci_addr.domain;
bus= dev-data.disk-controller_pci_addr.bus;
@@ -5463,19 +5429,92 @@ try_command:
if (domain == vm-def-controllers[i]-pci_addr.domain
bus== vm-def-controllers[i]-pci_addr.bus
slot == vm-def-controllers[i]-pci_addr.slot)
+conPtr = vm-def-controllers[i];
break;
}
-if (i == vm-def-ncontrollers) {
+if (!conPtr) {
+*ret = CONTROLLER_INEXISTENT;
+}
+}
+
+return conPtr;
+}
+
+static int qemudDomainAttachDiskDevice(virConnectPtr conn,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
+{
+int ret, i;
+char *cmd, *reply;
+char *safe_path;
+const char* type = NULL;
+int
[libvirt] [RFC] Interface for disk hotadd/remove
Hi,
I'm currently interested in implementing hard disk hot-add and -remove support
for qemu (as opposed to controller-based hotplugging), and this brings up the
question how to best support this feature in libvirt. Many SCSI-Controllers in
real machines, for instance, allow to add and remove disks (without adding or
removing the controller itself) while the system is up and running, so it
would be nice to emulate this in a virtual machine. I'm focusing on
qemu on the backend side, but the problem is not related to this
particular backend. Rather, the question is how to integrate such
a feature best into libvirt.
Before implementing the functionality, it would be great to hear the
community's opinion which route to take wrt. the interface. Essentially, I can
see two options:
- Naturally, there are virDomain{At,De}tachDevice, but the pair implements
drive-hotadding via adding a new controller with an attached hard disk to
the system. By extending the XML description of the drive with a parameter
that specifies whether controller- or disk-based hotplugging is to be
performed, it would be possible to implement the desired functionality,
whilst preserving compatibility with existing semantics. Removing the drive
would then require another new parameter in the XML description to identify
the drive on the controller, which does not really prettify the thing.
- Extend the API with a new method (for instance virDomainDiskAttach) that
takes a hard disk description, a controller identifier, and a parameter that
identifies the disk on the controller.
- (Theoretically, it would also be possible to implement media exchange for
hard disks in qemu and re-use the media exchange infrastructure already
present in libvirt for CD-ROMs, but since this possibility comes to use on
real hardware only very occasionally, guest operating systems are typically
not really prepared to handle this well)
My preference would be to go for option 2, that is, implement a new API
method. Would there be any obstacles against adding such a patch to
mainline? Or is anyone already working on similar functionality? Or can this
be done in a much simpler way I've missed? If not, then I'd send patches
for more detailed review before long.
Thanks,
Wolfgang
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Interface for disk hotadd/remove
Hi,
On Thu, Aug 13, 2009 at 8:31 PM, Daniel P. Berrangeberra...@redhat.com wrote:
On Thu, Aug 13, 2009 at 12:54:00PM +0200, Wolfgang Mauerer wrote:
I'm currently interested in implementing hard disk hot-add and -remove
support
for qemu (as opposed to controller-based hotplugging), and this brings up the
question how to best support this feature in libvirt. Many SCSI-Controllers
in
real machines, for instance, allow to add and remove disks (without adding or
removing the controller itself) while the system is up and running, so it
would be nice to emulate this in a virtual machine. I'm focusing on
qemu on the backend side, but the problem is not related to this
particular backend. Rather, the question is how to integrate such
a feature best into libvirt.
Before implementing the functionality, it would be great to hear the
community's opinion which route to take wrt. the interface. Essentially, I
can
see two options:
- Naturally, there are virDomain{At,De}tachDevice, but the pair implements
drive-hotadding via adding a new controller with an attached hard disk to
the system. By extending the XML description of the drive with a parameter
that specifies whether controller- or disk-based hotplugging is to be
performed, it would be possible to implement the desired functionality,
whilst preserving compatibility with existing semantics. Removing the drive
would then require another new parameter in the XML description to identify
the drive on the controller, which does not really prettify the thing.
- Extend the API with a new method (for instance virDomainDiskAttach) that
takes a hard disk description, a controller identifier, and a parameter that
identifies the disk on the controller.
I don't think its desirable to extend the API. The virDomainAttachDevice
API's XML parameter is intended to take any XML element that is valid
inside the domain devices XML section.
okay, that's good to know.
So the key to deciding how to deal with hotplug, is to first decide how
to represent disk controllers in the domain XML.
At boot time, if you list multiple SCSI disks in the XML, you get a single
controller with multiple disks attached. As such the current semantics of
the hotplug implementation for SCSI are divergant from the semantics at
boot time. This has the consequence that if you boot a guest with 2 SCSI
drives, then hot-attach one more you get 2 controllers, 3 disk. If you
then shutdown boot the guest again, you'll have 1 controller, 1 disks.
This says to me that the hotplug implementation for QEMU SCSI should be
fixed so that if you supply disk it adds a disk to the existing SCSI
controller. Obviously it requires some QEMU support for this first.
Qemu support for this is already there (I was not quite clear on that,
the thing I'm currently adding additionally is hot-remove). I completely
agree that the current semantics of libvirt-controlled disk-hotplug in qemu
provide some possibilities for improvement, but I was not sure if it's
okay to change the semantics of a libvirt command whilst preserving its syntax.
Thus my feeling would be todo something like adding a new controller
element to represent a disk controller. Support hotplugging controller
instances directly with obvious semantics.
Then extend the disk schema to allow a controller name/identifier to
be provided. If no controller is listed then plug the disk into the
first available controller slot, otherwise use the explicitly requested
controller. Never implicitily add new controllers, unless dealing with
a legacy QEMU without the disk hotplug support that you're writing.
Sounds reasonable to me. Roughly, that would lead to something along
the lines of (for a SCSI host on the PCI bus)
controller type=scsi id=my_controller
bus addr=00:04/
/controller
disk type=scsi
source file=disk.img/
controller id=my_controller unit=0/
/disk
This allows for specifying both the address of the controller on the
bus, and to identify the disk on the controller -- this is important for
hot-remove if multiple physically identical disks are attached to a single
controller.
If virDomainAttachDevice is fed with a disk element containing
an explicitely specified controller id, libvirt can then simply add the
disk if the controller exists, or add controller+disk if the controller
is not yet present on the machine. If the controller is not specified
and none is present in the system, follow the current behaviour. If
no controller is specified, but a controller is present, just add the
disk, not controller+disk, breaking current behaviour. For hot-remove,
do the reverse action (though I'll consider under which circumstances
it is desirable to remove the controller when a disk is supposed to be
removed, but these details are best discussed once I've done the
code, I suppose). Does what I've outlined correspond to your idea?
Thanks,
Wolfgang
--
Libvir-list mailing list
Libvir-list@redhat.com
