* [Qemu-devel] [PATCH 0/9] Support disk-hotremove and controller hotplugging
@ 2009-09-18 15:26 Wolfgang Mauerer
2009-09-18 15:26 ` [Qemu-devel] [PATCH 1/9] Clarify documentation for private symbols Wolfgang Mauerer
` (9 more replies)
0 siblings, 10 replies; 14+ messages in thread
From: Wolfgang Mauerer @ 2009-09-18 15:26 UTC (permalink / raw)
To: libvir-list; +Cc: jan.kiszka, qemu-devel, wlm.libvirt
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 1/9] Clarify documentation for private symbols
2009-09-18 15:26 [Qemu-devel] [PATCH 0/9] Support disk-hotremove and controller hotplugging Wolfgang Mauerer
@ 2009-09-18 15:26 ` Wolfgang Mauerer
2009-09-18 15:26 ` [Qemu-devel] [PATCH 2/9] Extend <disk> element with controller information Wolfgang Mauerer
` (8 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Wolfgang Mauerer @ 2009-09-18 15:26 UTC (permalink / raw)
To: libvir-list; +Cc: jan.kiszka, qemu-devel, wlm.libvirt
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.mauerer@siemens.com>
Signed-off-by: Jan Kiszka <jan.kiszka@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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 2/9] Extend <disk> element with controller information
2009-09-18 15:26 [Qemu-devel] [PATCH 0/9] Support disk-hotremove and controller hotplugging Wolfgang Mauerer
2009-09-18 15:26 ` [Qemu-devel] [PATCH 1/9] Clarify documentation for private symbols Wolfgang Mauerer
@ 2009-09-18 15:26 ` Wolfgang Mauerer
2009-09-18 15:26 ` [Qemu-devel] [PATCH 3/9] Add new domain device: "controller" Wolfgang Mauerer
` (7 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Wolfgang Mauerer @ 2009-09-18 15:26 UTC (permalink / raw)
To: libvir-list; +Cc: jan.kiszka, qemu-devel, wlm.libvirt
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.mauerer@siemens.com>
Signed-off-by: Jan Kiszka <jan.kiszka@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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 3/9] Add new domain device: "controller"
2009-09-18 15:26 [Qemu-devel] [PATCH 0/9] Support disk-hotremove and controller hotplugging Wolfgang Mauerer
2009-09-18 15:26 ` [Qemu-devel] [PATCH 1/9] Clarify documentation for private symbols Wolfgang Mauerer
2009-09-18 15:26 ` [Qemu-devel] [PATCH 2/9] Extend <disk> element with controller information Wolfgang Mauerer
@ 2009-09-18 15:26 ` Wolfgang Mauerer
2009-09-24 10:52 ` [Qemu-devel] Re: [libvirt] " Daniel P. Berrange
2009-09-18 15:26 ` [Qemu-devel] [PATCH 4/9] Add disk-based hotplugging for the qemu backend Wolfgang Mauerer
` (6 subsequent siblings)
9 siblings, 1 reply; 14+ messages in thread
From: Wolfgang Mauerer @ 2009-09-18 15:26 UTC (permalink / raw)
To: libvir-list; +Cc: jan.kiszka, qemu-devel, wlm.libvirt
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.mauerer@siemens.com>
Signed-off-by: Jan Kiszka <jan.kiszka@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>
+ <value>floppy</value>
+ <value>disk</value>
+ <value>cdrom</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">
+ <value>file</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">
+ <value>block</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>
- <value>floppy</value>
- <value>disk</value>
- <value>cdrom</value>
+ <value>ide</value>
+ <value>fdc</value>
+ <value>scsi</value>
+ <value>virtio</value>
+ <value>xen</value>
+ <value>usb</value>
+ <value>uml</value>
</choice>
</attribute>
</optional>
- <choice>
- <group>
- <attribute name="type">
- <value>file</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">
- <value>block</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 -->
+ <value>scsi</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>
+ </element>
+ </optional>
</element>
</define>
<define name="target">
diff --git a/src/domain_conf.c b/src/domain_conf.c
index a6120c8..bbaf944 100644
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -79,6 +79,7 @@ VIR_ENUM_IMPL(virDomainLifecycle, VIR_DOMAIN_LIFECYCLE_LAST,
VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
"disk",
+ "controller",
"filesystem",
"interface",
"input",
@@ -294,6 +295,18 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
VIR_FREE(def);
}
+void virDomainControllerDefFree(virDomainControllerDefPtr def)
+{
+ if (!def)
+ return;
+
+ VIR_FREE(def->type);
+ VIR_FREE(def->id);
+ VIR_FREE(def->pci_addr);
+
+ VIR_FREE(def);
+}
+
void virDomainFSDefFree(virDomainFSDefPtr def)
{
if (!def)
@@ -651,6 +664,7 @@ virDomainDiskDefParseXML(virConnectPtr conn,
char *source = NULL;
char *target = NULL;
char *controller = NULL;
+ char *controller_pci_addr = NULL;
char *bus_id = NULL;
char *unit_id = NULL;
char *controller_id = NULL;
@@ -709,6 +723,7 @@ virDomainDiskDefParseXML(virConnectPtr conn,
} else if ((controller == 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");
} else if ((driverName == NULL) &&
@@ -827,6 +842,19 @@ virDomainDiskDefParseXML(virConnectPtr conn,
_("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);
+ goto error;
+ }
}
if (def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY &&
@@ -895,6 +923,77 @@ cleanup:
}
+/* Parse the XML definition for a controller
+ * @param node XML nodeset to parse for controller definition
+ */
+static virDomainControllerDefPtr
+virDomainControllerDefParseXML(virConnectPtr conn,
+ xmlNodePtr node,
+ int flags ATTRIBUTE_UNUSED) {
+ virDomainControllerDefPtr def;
+ xmlNodePtr cur;
+ char *type = NULL;
+ char *bus_addr = NULL;
+
+ if (VIR_ALLOC(def) < 0) {
+ virReportOOMError(conn);
+ return NULL;
+ }
+
+ type = virXMLPropString(node, "type");
+ def->type = VIR_DOMAIN_DISK_BUS_SCSI;
+ if (type) {
+ if ((def->type = virDomainDiskBusTypeFromString(type)) < 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("unknown disk bus type '%s'"), type);
+ goto error;
+ }
+ }
+
+ def->id = virXMLPropString(node, "id");
+
+ cur = node->children;
+ while (cur != NULL) {
+ if (cur->type == XML_ELEMENT_NODE &&
+ bus_addr == NULL &&
+ xmlStrEqual(cur->name, BAD_CAST "bus")) {
+
+ bus_addr = virXMLPropString(cur, "addr");
+
+ def->pci_addr.domain = -1;
+ def->pci_addr.bus = -1;
+ def->pci_addr.slot = -1;
+ if (bus_addr &&
+ sscanf(bus_addr, "%x:%x:%x",
+ &def->pci_addr.domain,
+ &def->pci_addr.bus,
+ &def->pci_addr.slot) < 3) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ "Unable to parse <bus> 'addr' parameter '%s'",
+ bus_addr);
+ goto error;
+ }
+
+ VIR_DEBUG("Parse PCI address of controller as %d:%d:%d\n",
+ def->pci_addr.domain, def->pci_addr.bus,
+ def->pci_addr.slot);
+ }
+ cur = cur->next;
+ }
+
+
+cleanup:
+ VIR_FREE(type);
+ VIR_FREE(bus_addr);
+
+ return def;
+
+ error:
+ virDomainControllerDefFree(def);
+ def = NULL;
+ goto cleanup;
+}
+
/* Parse the XML definition for a disk
* @param node XML nodeset to parse for disk definition
*/
@@ -2375,6 +2474,11 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virConnectPtr conn,
dev->type = VIR_DOMAIN_DEVICE_DISK;
if (!(dev->data.disk = virDomainDiskDefParseXML(conn, node, flags)))
goto error;
+ } else if (xmlStrEqual(node->name, BAD_CAST "controller")) {
+ dev->type = VIR_DOMAIN_DEVICE_CONTROLLER;
+ if (!(dev->data.controller =
+ virDomainControllerDefParseXML(conn, node, flags)))
+ goto error;
} else if (xmlStrEqual(node->name, BAD_CAST "filesystem")) {
dev->type = VIR_DOMAIN_DEVICE_FS;
if (!(dev->data.fs = virDomainFSDefParseXML(conn, node, flags)))
@@ -2783,6 +2887,27 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn,
}
VIR_FREE(nodes);
+ /* analysis of the disk controllers */
+ if ((n = virXPathNodeSet(conn, "./devices/controller", ctxt, &nodes)) < 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ "%s", _("cannot extract controller devices"));
+ goto error;
+ }
+ if (n && VIR_ALLOC_N(def->controllers, n) < 0)
+ goto no_memory;
+ for (i = 0 ; i < n ; i++) {
+ virDomainControllerDefPtr controller =
+ virDomainControllerDefParseXML(conn, nodes[i], flags);
+ if (!controller)
+ goto error;
+
+ def->controllers[def->ncontrollers++] = controller;
+ }
+ /* qsort(def->controllers, def->ncontrollers, sizeof(*def->controllers),
+ virDomainControllerQSort); */
+ VIR_FREE(nodes);
+
+
/* analysis of the filesystems */
if ((n = virXPathNodeSet(conn, "./devices/filesystem", ctxt, &nodes)) < 0) {
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
diff --git a/src/domain_conf.h b/src/domain_conf.h
index 898f6c9..6b3cb09 100644
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -111,6 +111,11 @@ struct _virDomainDiskDef {
char *src;
char *dst;
char *controller_id;
+ struct {
+ unsigned domain;
+ unsigned bus;
+ unsigned slot;
+ } controller_pci_addr;
char *driverName;
char *driverType;
char *serial;
@@ -125,6 +130,19 @@ struct _virDomainDiskDef {
virStorageEncryptionPtr encryption;
};
+/* Stores the virtual disk controller configuration */
+typedef struct _virDomainControllerDef virDomainControllerDef;
+typedef virDomainControllerDef *virDomainControllerDefPtr;
+struct _virDomainControllerDef {
+ int type;
+ char *id;
+ struct {
+ unsigned domain;
+ unsigned bus;
+ unsigned slot;
+ } pci_addr;
+};
+
static inline int
virDiskHasValidPciAddr(virDomainDiskDefPtr def)
{
@@ -441,6 +459,7 @@ enum virDomainDeviceType {
VIR_DOMAIN_DEVICE_SOUND,
VIR_DOMAIN_DEVICE_VIDEO,
VIR_DOMAIN_DEVICE_HOSTDEV,
+ VIR_DOMAIN_DEVICE_CONTROLLER,
VIR_DOMAIN_DEVICE_LAST,
};
@@ -451,6 +470,7 @@ struct _virDomainDeviceDef {
int type;
union {
virDomainDiskDefPtr disk;
+ virDomainControllerDefPtr controller;
virDomainFSDefPtr fs;
virDomainNetDefPtr net;
virDomainInputDefPtr input;
@@ -561,6 +581,9 @@ struct _virDomainDef {
int ndisks;
virDomainDiskDefPtr *disks;
+ int ncontrollers;
+ virDomainControllerDefPtr *controllers;
+
int nfss;
virDomainFSDefPtr *fss;
@@ -637,6 +660,7 @@ virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms,
void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def);
void virDomainInputDefFree(virDomainInputDefPtr def);
void virDomainDiskDefFree(virDomainDiskDefPtr def);
+void virDomainControllerDefFree(virDomainControllerDefPtr def);
void virDomainFSDefFree(virDomainFSDefPtr def);
void virDomainNetDefFree(virDomainNetDefPtr def);
void virDomainChrDefFree(virDomainChrDefPtr def);
--
1.6.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 4/9] Add disk-based hotplugging for the qemu backend
2009-09-18 15:26 [Qemu-devel] [PATCH 0/9] Support disk-hotremove and controller hotplugging Wolfgang Mauerer
` (2 preceding siblings ...)
2009-09-18 15:26 ` [Qemu-devel] [PATCH 3/9] Add new domain device: "controller" Wolfgang Mauerer
@ 2009-09-18 15:26 ` Wolfgang Mauerer
2009-09-18 15:26 ` [Qemu-devel] [PATCH 5/9] Implement controller hotplugging Wolfgang Mauerer
` (5 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Wolfgang Mauerer @ 2009-09-18 15:26 UTC (permalink / raw)
To: libvir-list; +Cc: jan.kiszka, qemu-devel, wlm.libvirt
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.mauerer@siemens.com>
Signed-off-by: Jan Kiszka <jan.kiszka@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 prints:
* OK bus 0, slot XXX...
@@ -5322,16 +5322,70 @@ qemudParsePciAddReply(virDomainObjPtr vm,
return 0;
}
-static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
- virDomainObjPtr vm,
- virDomainDeviceDefPtr dev)
+static int
+qemudParseDriveAddReply(virDomainObjPtr vm,
+ const char *reply,
+ int *bus,
+ int *unit)
+{
+ char *s, *e;
+
+ DEBUG("%s: drive_add reply: %s", vm->def->name, reply);
+
+ /* If the command succeeds qemu prints:
+ * OK bus X, unit Y
+ */
+
+ if (!(s = strstr(reply, "OK ")))
+ return -1;
+
+ s += 3;
+
+ if (STRPREFIX(s, "bus ")) {
+ s += strlen("bus ");
+
+ if (virStrToLong_i(s, &e, 10, bus) == -1) {
+ VIR_WARN(_("Unable to parse bus '%s'\n"), s);
+ return -1;
+ }
+
+ if (!STRPREFIX(e, ", ")) {
+ VIR_WARN(_("Expected ', ' parsing drive_add reply '%s'\n"), s);
+ return -1;
+ }
+ s = e + 2;
+ }
+
+ if (!STRPREFIX(s, "unit ")) {
+ VIR_WARN(_("Expected 'unit ' parsing drive_add reply '%s'\n"), s);
+ return -1;
+ }
+ s += strlen("bus ");
+
+ if (virStrToLong_i(s, &e, 10, unit) == -1) {
+ VIR_WARN(_("Unable to parse unit number '%s'\n"), s);
+ return -1;
+ }
+
+ 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)) {
@@ -5353,8 +5407,48 @@ try_command:
return -1;
}
- ret = virAsprintf(&cmd, "pci_add %s storage file=%s,if=%s",
- (tryOldSyntax ? "0": "pci_addr=auto"), safe_path, type);
+ controller_specified = 0;
+ /* Partially specified PCI addresses (should not happen, anyway)
+ don't count as specification */
+ if (dev->data.disk->controller_id != NULL ||
+ (dev->data.disk->controller_pci_addr.domain != -1 &&
+ dev->data.disk->controller_pci_addr.bus != -1 &&
+ dev->data.disk->controller_pci_addr.slot != -1)) {
+ controller_specified = 1;
+ }
+
+ 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->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 %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",
+ (tryOldSyntax ? "" : "pci_addr="), domain, bus,
+ slot, safe_path, type, bus_unit_string);
+ }
+ else {
+ /* Old-style behaviour: If no <controller> reference is given, then
+ hotplug a new controller with the disk attached. */
+ VIR_DEBUG ("%s: pci_add %s storage file=%s,if=%s", vm->def->name,
+ (tryOldSyntax ? "0": "pci_addr=auto"), safe_path, type);
+ 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);
@@ -5370,26 +5464,149 @@ try_command:
VIR_FREE(cmd);
- if (qemudParsePciAddReply(vm, reply, &domain, &bus, &slot) < 0) {
- if (!tryOldSyntax && strstr(reply, "invalid char in expression")) {
- VIR_FREE(reply);
- tryOldSyntax = 1;
- goto try_command;
+ if (controller_specified) {
+ 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;
+ }
+ } else {
+ 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 disk failed: %s"), type, reply);
+ VIR_FREE(reply);
+ return -1;
+ }
+ }
+
+ 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);
+
+
+ return 0;
+}
+
+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;
+ int controllerAddressSpecified = 0;
+ unsigned domain, bus, slot;
+
+ /* Test if the controller already exists, both by ID and
+ by PCI address */
+ for (i = 0 ; i < vm->def->ncontrollers ; i++) {
+ if ((dev->data.controller->id &&
+ STREQ(vm->def->controllers[i]->id, dev->data.controller->id)) ||
+ (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)) {
+ qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ _("Controller (id:%s, %d:%d:%d) already exists"),
+ dev->data.controller->id ?
+ dev->data.controller->id : "(none)",
+ dev->data.controller->pci_addr.domain,
+ dev->data.controller->pci_addr.bus,
+ dev->data.controller->pci_addr.slot);
+ return -1;
}
+ }
- qemudReportError (conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _("adding %s disk failed: %s"), type, reply);
- VIR_FREE(reply);
+ if (VIR_REALLOC_N(vm->def->controllers, vm->def->ncontrollers+1) < 0) {
+ virReportOOMError(conn);
return -1;
}
- VIR_FREE(reply);
+ if (dev->data.controller->pci_addr.domain != -1 &&
+ dev->data.controller->pci_addr.bus != -1 &&
+ dev->data.controller->pci_addr.slot != -1) {
+ controllerAddressSpecified = 1;
+ }
- dev->data.disk->pci_addr.domain = domain;
- dev->data.disk->pci_addr.bus = bus;
- dev->data.disk->pci_addr.slot = slot;
+try_command:
+ if (controllerAddressSpecified) {
+ domain = dev->data.controller->pci_addr.domain;
+ bus = dev->data.controller->pci_addr.bus;
+ slot = dev->data.controller->pci_addr.slot;
+
+ ret = virAsprintf(&tmp, "%d:%d:%d", 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);
+ }
- virDomainDiskInsertPreAlloced(vm->def, dev->data.disk);
+ 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);
+
+ 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 (TODO: Can this really happen?) */
+ 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);*/
return 0;
}
@@ -5868,7 +6085,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
ret = qemudDomainAttachUsbMassstorageDevice(dom->conn, vm, dev);
} else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI ||
dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
- ret = qemudDomainAttachPciDiskDevice(dom->conn, vm, dev);
+ ret = qemudDomainAttachDiskDevice(dom->conn, vm, dev);
} else {
qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
_("disk bus '%s' cannot be hotplugged."),
--
1.6.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 5/9] Implement controller hotplugging
2009-09-18 15:26 [Qemu-devel] [PATCH 0/9] Support disk-hotremove and controller hotplugging Wolfgang Mauerer
` (3 preceding siblings ...)
2009-09-18 15:26 ` [Qemu-devel] [PATCH 4/9] Add disk-based hotplugging for the qemu backend Wolfgang Mauerer
@ 2009-09-18 15:26 ` Wolfgang Mauerer
2009-09-18 15:26 ` [Qemu-devel] [PATCH 6/9] Allow controller selection by ID Wolfgang Mauerer
` (4 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Wolfgang Mauerer @ 2009-09-18 15:26 UTC (permalink / raw)
To: libvir-list; +Cc: jan.kiszka, qemu-devel, wlm.libvirt
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.mauerer@siemens.com>
Signed-off-by: Jan Kiszka <jan.kiszka@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 @@ 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 6/9] Allow controller selection by ID
2009-09-18 15:26 [Qemu-devel] [PATCH 0/9] Support disk-hotremove and controller hotplugging Wolfgang Mauerer
` (4 preceding siblings ...)
2009-09-18 15:26 ` [Qemu-devel] [PATCH 5/9] Implement controller hotplugging Wolfgang Mauerer
@ 2009-09-18 15:26 ` Wolfgang Mauerer
2009-09-18 15:26 ` [Qemu-devel] [PATCH 7/9] Remove surprises in the semantics of disk-hotadd Wolfgang Mauerer
` (3 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Wolfgang Mauerer @ 2009-09-18 15:26 UTC (permalink / raw)
To: libvir-list; +Cc: jan.kiszka, qemu-devel, wlm.libvirt
... by simply traversing the list of controllers
to find the associated PCI address.
Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@siemens.com>
Signed-off-by: Jan Kiszka <jan.kiszka@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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 7/9] Remove surprises in the semantics of disk-hotadd
2009-09-18 15:26 [Qemu-devel] [PATCH 0/9] Support disk-hotremove and controller hotplugging Wolfgang Mauerer
` (5 preceding siblings ...)
2009-09-18 15:26 ` [Qemu-devel] [PATCH 6/9] Allow controller selection by ID Wolfgang Mauerer
@ 2009-09-18 15:26 ` Wolfgang Mauerer
2009-09-24 10:59 ` [Qemu-devel] Re: [libvirt] " Daniel P. Berrange
2009-09-18 15:26 ` [Qemu-devel] [PATCH 8/9] Factor out the method to get the PCI address of a controller for a given disk Wolfgang Mauerer
` (2 subsequent siblings)
9 siblings, 1 reply; 14+ messages in thread
From: Wolfgang Mauerer @ 2009-09-18 15:26 UTC (permalink / raw)
To: libvir-list; +Cc: jan.kiszka, qemu-devel, wlm.libvirt
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.mauerer@siemens.com>
Signed-off-by: Jan Kiszka <jan.kiszka@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, bus_unit_string);
- ret = virAsprintf(&cmd, "drive_add %s%d:%d:%d file=%s,if=%s%s",
- (tryOldSyntax ? "" : "pci_addr="), domain, bus,
- slot, safe_path, type, bus_unit_string);
+ /* At this point, we have a valid (domain, bus, slot) triple
+ that identifies the controller, regardless if an explicit
+ controller has been given or not. */
+ if (dev->data.disk->bus_id != -1) {
+ virBufferVSprintf(&buf, ",bus=%d", dev->data.disk->bus_id);
}
- else {
- /* Old-style behaviour: If no <controller> reference is given, then
- hotplug a new controller with the disk attached. */
- VIR_DEBUG ("%s: pci_add %s storage file=%s,if=%s", vm->def->name,
- (tryOldSyntax ? "0": "pci_addr=auto"), safe_path, type);
- ret = virAsprintf(&cmd, "pci_add %s storage file=%s,if=%s",
- (tryOldSyntax ? "0": "pci_addr=auto"), safe_path, type);
+
+ 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 %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",
+ (tryOldSyntax ? "" : "pci_addr="), domain, bus,
+ slot, safe_path, type, bus_unit_string);
+
VIR_FREE(safe_path);
if (ret == -1) {
virReportOOMError(conn);
@@ -5494,41 +5507,26 @@ try_command:
VIR_FREE(cmd);
- if (controller_specified) {
- 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;
- }
- } else {
- 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 disk failed: %s"), type, reply);
- VIR_FREE(reply);
- return -1;
- }
+ 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;
--
1.6.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 8/9] Factor out the method to get the PCI address of a controller for a given disk
2009-09-18 15:26 [Qemu-devel] [PATCH 0/9] Support disk-hotremove and controller hotplugging Wolfgang Mauerer
` (6 preceding siblings ...)
2009-09-18 15:26 ` [Qemu-devel] [PATCH 7/9] Remove surprises in the semantics of disk-hotadd Wolfgang Mauerer
@ 2009-09-18 15:26 ` Wolfgang Mauerer
2009-09-18 15:26 ` [Qemu-devel] [PATCH 9/9] Implement disk- and controller hotremove Wolfgang Mauerer
[not found] ` <20091009163203.GA30218@redhat.com>
9 siblings, 0 replies; 14+ messages in thread
From: Wolfgang Mauerer @ 2009-09-18 15:26 UTC (permalink / raw)
To: libvir-list; +Cc: jan.kiszka, qemu-devel, wlm.libvirt
We need this multiple times later on.
Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@siemens.com>
Signed-off-by: Jan Kiszka <jan.kiszka@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 tryOldSyntax = 0;
+ int bus_id, unit_id;
+ int domain, bus, slot;
+ virDomainControllerDefPtr conPtr;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ char* bus_unit_string;
+
+ /* Both controller and disk can specify a type like SCSI. While
+ a virtual disk as such is not bound to a specific bus type,
+ the specification is here for historical reasons. When controller
+ and disk type specification disagree, the former takes precedence
+ and we print a warning message, but still add the disk. */
+ if (virDiskHasValidController(dev->data.disk)) {
+ conPtr = qemudGetDiskController(vm, dev, &ret);
+ if (conPtr && dev->data.disk->bus != conPtr->type) {
+ VIR_WARN0(_("Controller and disk bus type disagree, controller " \
+ "takes precedence\n"));
+ type = virDomainDiskBusTypeToString(conPtr->type);
+ }
+ }
+ if (!type) {
+ 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;
+ }
+
+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) {
+ case CONTROLLER_INEXISTENT:
qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
_("Controller does not exist"));
return -1;
+ break; /* A bit pointless */
+ case NO_CONTROLLER_FOUND:
+ qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ _("Cannot find appropriate controller for hot-add"));
+ return -1;
+ break;
}
}
- /* At this point, we have a valid (domain, bus, slot) triple
- that identifies the controller, regardless if an explicit
- controller has been given or not. */
+ /* At this point, we have a valid controller, regardless if an explicit
+ controller has been specified or not. */
+ domain = conPtr->pci_addr.domain;
+ bus = conPtr->pci_addr.bus;
+ slot = conPtr->pci_addr.slot;
+
if (dev->data.disk->bus_id != -1) {
virBufferVSprintf(&buf, ",bus=%d", dev->data.disk->bus_id);
}
--
1.6.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 9/9] Implement disk- and controller hotremove
2009-09-18 15:26 [Qemu-devel] [PATCH 0/9] Support disk-hotremove and controller hotplugging Wolfgang Mauerer
` (7 preceding siblings ...)
2009-09-18 15:26 ` [Qemu-devel] [PATCH 8/9] Factor out the method to get the PCI address of a controller for a given disk Wolfgang Mauerer
@ 2009-09-18 15:26 ` Wolfgang Mauerer
[not found] ` <20091009163203.GA30218@redhat.com>
9 siblings, 0 replies; 14+ messages in thread
From: Wolfgang Mauerer @ 2009-09-18 15:26 UTC (permalink / raw)
To: libvir-list; +Cc: jan.kiszka, qemu-devel, wlm.libvirt
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.mauerer@siemens.com>
Signed-off-by: Jan Kiszka <jan.kiszka@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->data.disk->bus_id == vm->def->disks[i]->bus_id &&
+ dev->data.disk->unit_id != -1 &&
+ dev->data.disk->unit_id == vm->def->disks[i]->unit_id) {
+ detach = vm->def->disks[i];
+ break;
+ }
+ }
+ }
+
+ /* If the device did not specify a controller explicitely (and therefore
+ lacks a bus and unit specification), revert to the explicit target
+ device specification to identify a device for removal. */
+ if (!detach) {
+ 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) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ _("disk %s not found"), dev->data.disk->dst);
+ goto cleanup;
+ }
+
+ if (!virDiskHasValidPciAddr(detach) && !virDiskHasValidController(detach)) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ _("disk %s cannot be detached - no PCI address for device"),
+ detach->dst);
+ goto cleanup;
+ }
+
+ type = virDomainDiskBusTypeToString(detach->bus);
+
+try_command:
+ if (tryOldSyntax) {
+ if (virAsprintf(&cmd, "drive_del 0 %d:%d:%d bus=%d,unit=%d,if=%s",
+ detach->pci_addr.domain, detach->pci_addr.bus,
+ detach->pci_addr.slot, detach->bus_id,
+ detach->unit_id, type) < 0) {
+ virReportOOMError(conn);
+ goto cleanup;
+ }
+ } else {
+ if (virAsprintf(&cmd, "drive_del pci_addr=%d:%d:%d bus=%d,unit=%d,if=%s",
+ detach->pci_addr.domain,
+ detach->pci_addr.bus,
+ detach->pci_addr.slot, detach->bus_id,
+ detach->unit_id, type) < 0) {
+ virReportOOMError(conn);
+ goto cleanup;
+ }
+ }
+
+ if (qemudMonitorCommand(vm, cmd, &reply) < 0) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ _("failed to execute detach disk %s command"), detach->dst);
+ goto cleanup;
+ }
+
+ DEBUG ("%s: drive_del reply: %s",vm->def->name, reply);
+
+ if (!tryOldSyntax &&
+ strstr(reply, "extraneous characters")) {
+ tryOldSyntax = 1;
+ goto try_command;
+ }
+ /* OK bux x, unit x is printed on success, but this does not provide
+ any new information to us.*/
+ if (strstr(reply, "Invalid pci address") ||
+ strstr(reply, "No pci device with address")) {
+ qemudReportError (conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
_("failed to detach disk %s: invalid PCI address %.4x:%.2x:%.2x: %s"),
detach->dst,
detach->pci_addr.domain,
@@ -6274,6 +6393,11 @@ try_command:
detach->pci_addr.slot,
reply);
goto cleanup;
+ } else if (strstr(reply, "Need to specify bus") ||
+ strstr(reply, "Need to specify unit")) {
+ qemudReportError (conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ _("failed to detach disk %s: bus and unit not (internal error)"),
+ detach->dst);
}
if (vm->def->ndisks > 1) {
@@ -6575,7 +6699,7 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK &&
(dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI ||
dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO)) {
- ret = qemudDomainDetachPciDiskDevice(dom->conn, vm, dev);
+ ret = qemudDomainDetachDiskDevice(dom->conn, vm, dev);
if (driver->securityDriver)
driver->securityDriver->domainRestoreSecurityImageLabel(dom->conn, dev->data.disk);
if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 1) < 0)
@@ -6584,6 +6708,11 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
ret = qemudDomainDetachNetDevice(dom->conn, vm, dev);
} else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) {
ret = qemudDomainDetachHostDevice(dom->conn, driver, vm, dev);
+ } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) {
+ /* NOTE: We can unplug the controller without having to
+ check if all disks are unplugged; it is at the user's
+ responsibility to use the required OS services first. */
+ ret = qemudDomainDetachDiskController(dom->conn, vm, dev);
} else
qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
"%s", _("only SCSI or virtio disk device can be detached dynamically"));
--
1.6.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] Re: [libvirt] [PATCH 3/9] Add new domain device: "controller"
2009-09-18 15:26 ` [Qemu-devel] [PATCH 3/9] Add new domain device: "controller" Wolfgang Mauerer
@ 2009-09-24 10:52 ` Daniel P. Berrange
0 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2009-09-24 10:52 UTC (permalink / raw)
To: Wolfgang Mauerer; +Cc: libvir-list, jan.kiszka, qemu-devel
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='0x0000' 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.
> diff --git a/src/domain_conf.h b/src/domain_conf.h
> index 898f6c9..6b3cb09 100644
> --- a/src/domain_conf.h
> +++ b/src/domain_conf.h
> @@ -111,6 +111,11 @@ struct _virDomainDiskDef {
> char *src;
> char *dst;
> char *controller_id;
> + struct {
> + unsigned domain;
> + unsigned bus;
> + unsigned slot;
> + } controller_pci_addr;
I think we should stick to just using the controller name
as the mandatory identifier for cross-referencing disks
to controllers.
> char *driverName;
> char *driverType;
> char *serial;
> @@ -125,6 +130,19 @@ struct _virDomainDiskDef {
> virStorageEncryptionPtr encryption;
> };
>
> +/* Stores the virtual disk controller configuration */
> +typedef struct _virDomainControllerDef virDomainControllerDef;
> +typedef virDomainControllerDef *virDomainControllerDefPtr;
> +struct _virDomainControllerDef {
> + int type;
> + char *id;
> + struct {
> + unsigned domain;
> + unsigned bus;
> + unsigned slot;
> + } pci_addr;
> +};
With the generic address data type and s/id/name/, this would be just
struct _virDomainControllerDef {
int type;
char *name;
virDomainDeviceAddress addr;
};
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] Re: [libvirt] [PATCH 7/9] Remove surprises in the semantics of disk-hotadd
2009-09-18 15:26 ` [Qemu-devel] [PATCH 7/9] Remove surprises in the semantics of disk-hotadd Wolfgang Mauerer
@ 2009-09-24 10:59 ` Daniel P. Berrange
0 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2009-09-24 10:59 UTC (permalink / raw)
To: Wolfgang Mauerer; +Cc: libvir-list, jan.kiszka, qemu-devel
On Fri, Sep 18, 2009 at 05:26:14PM +0200, Wolfgang Mauerer wrote:
> 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)
Yes, I still think is the good way to go
Daniel
>
> Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@siemens.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@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, bus_unit_string);
> - ret = virAsprintf(&cmd, "drive_add %s%d:%d:%d file=%s,if=%s%s",
> - (tryOldSyntax ? "" : "pci_addr="), domain, bus,
> - slot, safe_path, type, bus_unit_string);
> + /* At this point, we have a valid (domain, bus, slot) triple
> + that identifies the controller, regardless if an explicit
> + controller has been given or not. */
> + if (dev->data.disk->bus_id != -1) {
> + virBufferVSprintf(&buf, ",bus=%d", dev->data.disk->bus_id);
> }
> - else {
> - /* Old-style behaviour: If no <controller> reference is given, then
> - hotplug a new controller with the disk attached. */
> - VIR_DEBUG ("%s: pci_add %s storage file=%s,if=%s", vm->def->name,
> - (tryOldSyntax ? "0": "pci_addr=auto"), safe_path, type);
> - ret = virAsprintf(&cmd, "pci_add %s storage file=%s,if=%s",
> - (tryOldSyntax ? "0": "pci_addr=auto"), safe_path, type);
> +
> + 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 %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",
> + (tryOldSyntax ? "" : "pci_addr="), domain, bus,
> + slot, safe_path, type, bus_unit_string);
> +
> VIR_FREE(safe_path);
> if (ret == -1) {
> virReportOOMError(conn);
> @@ -5494,41 +5507,26 @@ try_command:
>
> VIR_FREE(cmd);
>
> - if (controller_specified) {
> - 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;
> - }
> - } else {
> - 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 disk failed: %s"), type, reply);
> - VIR_FREE(reply);
> - return -1;
> - }
> + 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;
> --
> 1.6.4
>
> --
> Libvir-list mailing list
> Libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] Re: [libvirt] [PATCH 0/9] Support disk-hotremove and controller hotplugging
[not found] ` <20091009163203.GA30218@redhat.com>
@ 2009-10-13 9:08 ` Gerd Hoffmann
2009-10-13 9:34 ` Wolfgang Mauerer
0 siblings, 1 reply; 14+ messages in thread
From: Gerd Hoffmann @ 2009-10-13 9:08 UTC (permalink / raw)
To: veillard; +Cc: libvir-list, jan.kiszka, Wolfgang Mauerer, qemu-devel
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 !
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+).
cheers,
Gerd
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] Re: [libvirt] [PATCH 0/9] Support disk-hotremove and controller hotplugging
2009-10-13 9:08 ` [Qemu-devel] Re: [libvirt] [PATCH 0/9] Support disk-hotremove and controller hotplugging Gerd Hoffmann
@ 2009-10-13 9:34 ` Wolfgang Mauerer
0 siblings, 0 replies; 14+ messages in thread
From: Wolfgang Mauerer @ 2009-10-13 9:34 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: libvir-list, Kiszka, Jan, qemu-devel
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
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2009-10-13 9:32 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-18 15:26 [Qemu-devel] [PATCH 0/9] Support disk-hotremove and controller hotplugging Wolfgang Mauerer
2009-09-18 15:26 ` [Qemu-devel] [PATCH 1/9] Clarify documentation for private symbols Wolfgang Mauerer
2009-09-18 15:26 ` [Qemu-devel] [PATCH 2/9] Extend <disk> element with controller information Wolfgang Mauerer
2009-09-18 15:26 ` [Qemu-devel] [PATCH 3/9] Add new domain device: "controller" Wolfgang Mauerer
2009-09-24 10:52 ` [Qemu-devel] Re: [libvirt] " Daniel P. Berrange
2009-09-18 15:26 ` [Qemu-devel] [PATCH 4/9] Add disk-based hotplugging for the qemu backend Wolfgang Mauerer
2009-09-18 15:26 ` [Qemu-devel] [PATCH 5/9] Implement controller hotplugging Wolfgang Mauerer
2009-09-18 15:26 ` [Qemu-devel] [PATCH 6/9] Allow controller selection by ID Wolfgang Mauerer
2009-09-18 15:26 ` [Qemu-devel] [PATCH 7/9] Remove surprises in the semantics of disk-hotadd Wolfgang Mauerer
2009-09-24 10:59 ` [Qemu-devel] Re: [libvirt] " Daniel P. Berrange
2009-09-18 15:26 ` [Qemu-devel] [PATCH 8/9] Factor out the method to get the PCI address of a controller for a given disk Wolfgang Mauerer
2009-09-18 15:26 ` [Qemu-devel] [PATCH 9/9] Implement disk- and controller hotremove Wolfgang Mauerer
[not found] ` <20091009163203.GA30218@redhat.com>
2009-10-13 9:08 ` [Qemu-devel] Re: [libvirt] [PATCH 0/9] Support disk-hotremove and controller hotplugging Gerd Hoffmann
2009-10-13 9:34 ` Wolfgang Mauerer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.