4:22 p.m.
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.
Wolfgang's series had added new element for SCSI controllers,
with PCI address info about the controller
<controller type='ide' index='0'>
<address domain='0x0000' bus='0x00' slot='0x01'
function='0x1'/>
</controller>
And had also extended the <disk> element to include a SCSI
controller name, and bus/unit ID. eg
<disk>
...
<controller name="<identifier>"
pci_addr="<addr>" bus="<number>"
unit="<number>"/>
</disk>
I then remembered that for support NIC/VirtIO/hostdev disk unplug
Mark M had previously added PCI address information to the internal
XML state files for <interface>, <disk> and <hostdev> elements.
All of these places using PCI addresses suffered from the fact
that we only knew the addresses of devices we'd hotplug and had no
idea of addresses of devices present at boot time.
A further issue with the addition of <controller> to the <disk>
element, is that not all disk types have a concept of a controller.
For example, every VirtIO disk is in fact a full PCI device, so
having a <controller> element in <disk> is not meaningful for
VirtIO.
The solution that I believe solves all our problems, is to add a
generic <address> element to every single device. This address
element contains info about how the device is associated with the
logical parent device. There will be three types of address in
this series of patches, though we could imagine adding more later.
- PCI address - domain, bus, slot, function
- USB address - bus, device
- Drive address - controller, bus, unit
Anything that is a PCI device will obviously use PCI addresses.
- PCI: sound, audio, video, all virtio, watchdog, disk controllers
- USB: all usb devices
- Drive: SCSI, IDE & Floppy disks, but *not* VirtIO/Xen disks
Xen paravirt devices aren't really covered in this scheme. I
could imagine adding a fourth address type for Xen. This would in
fact let us handle driver domains - ie a backend outside dom0.
I won't deal with Xen in this series though.
The XML for each address type looks like
<address type='pci' mode='static' domain='0x0000'
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.
libvirt itself will auto-assign all drive addresses, and QEMU
will auto-assign PCI adresses in dynamic mode. When starting
a guest VM we run 'info pci' to get a list of PCI vendor/product
IDs and matching PCI addresses. We then attempt to match those
up with the devices we specified to QEMU. It sounds nasty, but
it actually works fairly well. This means we also now make it
possible to hotunplug any device, even those the VM was initially
booted with
There are two ways I can envisage mgmt apps using this address
functionality
- Boot a guest with no addresses specified, grab the XML and
change all 'dynamic' attrs to 'static' and then define the
persistent config with this. The addresses will then be
unchanged forever more
- Explicitly give a full list of addresses the very first time
a guest is created.
There is one small issue with all this, we need to know every
PCI device in the guest. It turns out there are a handful of
devices in QEMU we don't represent in XML yet
- Virtio balloon device
- Virtio console (this is easy to address with matt's patches)
- ISA bridge
- USB controller
- Some kind of PCI bridge (not clear what this is, it has
PCI ID 8086:7113
If an management application is to be able to fully control
static PCI addressing, we need to represent these somehow,
so apps can give them addresses. Technically we could get
away with not representing the ISA/PCI bridge since QEMU
always gives them the first PCI slot no matter what. Still
need the VirtIO & USB devices dealt with.
Finally, here is an example of a guest running with a huge
number of devices. Notice how we've auto-detected the PCI
address of every device, and every disk. In particular
notice how VirtiO disks got the PCI address, while SCSI
disks got the drive address.
<domain type='kvm' id='2'>
<name>plain</name>
<uuid>c7a1edbd-edaf-9455-926a-d65c16db1809</uuid>
<memory>219200</memory>
<currentMemory>219136</currentMemory>
<vcpu>1</vcpu>
<os>
<type arch='i686' machine='pc-0.11'>hvm</type>
<kernel>/home/berrange/vmlinuz-PAE</kernel>
<initrd>/home/berrange/initrd-PAE.img</initrd>
<boot dev='hd'/>
</os>
<features>
<acpi/>
</features>
<clock offset='utc'/>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
<on_crash>destroy</on_crash>
<devices>
<emulator>/usr/bin/qemu</emulator>
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2'/>
<source file='/home/berrange/VirtualMachines/plain.qcow'/>
<target dev='vda' bus='virtio'/>
<address type='pci' mode='dynamic' domain='0x0000'
bus='0x00' slot='0x0b' function='0x0'/>
</disk>
<disk type='file' device='cdrom'>
<source file='/home/berrange/gpxe.iso'/>
<target dev='hdc' bus='ide'/>
<readonly/>
<address type='drive' mode='dynamic' controller='0'
bus='1' unit='0'/>
</disk>
<disk type='file' device='disk'>
<source file='/home/berrange/output.img'/>
<target dev='sda' bus='scsi'/>
<address type='drive' mode='dynamic' controller='0'
bus='0' unit='0'/>
</disk>
<disk type='file' device='disk'>
<source file='/home/berrange/output.img'/>
<target dev='sdd' bus='scsi'/>
<address type='drive' mode='dynamic' controller='0'
bus='0' unit='3'/>
</disk>
<disk type='file' device='disk'>
<source file='/home/berrange/output.img'/>
<target dev='sdf' bus='scsi'/>
<address type='drive' mode='dynamic' controller='0'
bus='0' unit='5'/>
</disk>
<controller type='scsi' index='0'>
<address type='pci' mode='dynamic' domain='0x0000'
bus='0x00' slot='0x0a' function='0x0'/>
</controller>
<controller type='ide' index='0'>
<address type='pci' mode='dynamic' domain='0x0000'
bus='0x00' slot='0x01' function='0x1'/>
</controller>
<controller type='fdc' index='0'/>
<interface type='user'>
<mac address='52:54:00:5b:ef:21'/>
<model type='ne2k_pci'/>
<address type='pci' mode='dynamic' domain='0x0000'
bus='0x00' slot='0x03' function='0x0'/>
</interface>
<interface type='user'>
<mac address='52:54:00:1c:dc:98'/>
<model type='virtio'/>
<address type='pci' mode='dynamic' domain='0x0000'
bus='0x00' slot='0x04' function='0x0'/>
</interface>
<interface type='user'>
<mac address='52:54:00:f7:c5:0e'/>
<model type='e1000'/>
<address type='pci' mode='dynamic' domain='0x0000'
bus='0x00' slot='0x05' function='0x0'/>
</interface>
<interface type='user'>
<mac address='52:54:00:56:6c:55'/>
<model type='pcnet'/>
<address type='pci' mode='dynamic' domain='0x0000'
bus='0x00' slot='0x06' function='0x0'/>
</interface>
<interface type='user'>
<mac address='52:54:00:ca:0d:58'/>
<model type='rtl8139'/>
<address type='pci' mode='dynamic' domain='0x0000'
bus='0x00' slot='0x07' function='0x0'/>
</interface>
<serial type='pty'>
<source path='/dev/pts/5'/>
<target port='0'/>
</serial>
<console type='pty' tty='/dev/pts/5'>
<source path='/dev/pts/5'/>
<target port='0'/>
</console>
<input type='mouse' bus='ps2'/>
<graphics type='vnc' port='5900' autoport='yes'/>
<sound model='ac97'>
<address type='pci' mode='dynamic' domain='0x0000'
bus='0x00' slot='0x08' function='0x0'/>
</sound>
<sound model='es1370'>
<address type='pci' mode='dynamic' domain='0x0000'
bus='0x00' slot='0x09' function='0x0'/>
</sound>
<video>
<model type='cirrus' vram='9216' heads='1'/>
<address type='pci' mode='dynamic' domain='0x0000'
bus='0x00' slot='0x02' function='0x0'/>
</video>
<watchdog model='i6300esb' action='reset'>
<address type='pci' mode='dynamic' domain='0x0000'
bus='0x00' slot='0x0c' function='0x0'/>
</watchdog>
</devices>
<seclabel type='dynamic' model='selinux'>
<label>system_u:system_r:svirt_t:s0:c181,c286</label>
<imagelabel>system_u:object_r:svirt_image_t:s0:c181,c286</imagelabel>
</seclabel>
</domain>
Daniel
4:22 p.m.
New subject: [libvirt] [PATCH 01/12] Introduce a standardized data structure for device addresses
All guest devices now use a common device address structure
summarized by:
enum virDomainDeviceAddressType {
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI,
};
enum virDomainDeviceAddressMode {
VIR_DOMAIN_DEVICE_ADDRESS_MODE_DYNAMIC,
VIR_DOMAIN_DEVICE_ADDRESS_MODE_STATIC,
};
struct _virDomainDevicePCIAddress {
unsigned int domain;
unsigned int bus;
unsigned int slot;
unsigned int function;
};
struct _virDomainDeviceAddress {
int type;
int mode;
union {
virDomainDevicePCIAddress pci;
} data;
};
This replaces the anonymous structs in Disk/Net/Hostdev data
structures. Where available, the address is *always* printed
in the XML file, instead of being hidden in the internal state
file.
<address type='pci' mode='static' domain='0x0000'
bus='0x1e' slot='0x07' function='0x0'/>
A 'static' address is one assigned by the user, never changing,
while a 'dynamic' address is one assigned on the fly by QEMU.
'dynamic' addresses are thrown away when a VM stops.
The structure definition is based on Wolfgang Mauerer's disk
controller patch series.
---
src/conf/domain_conf.c | 432 ++++++++++++++++++++++++++++++++++-----------
src/conf/domain_conf.h | 90 ++++++-----
src/libvirt_private.syms | 5 +
src/qemu/qemu_driver.c | 64 ++++---
4 files changed, 418 insertions(+), 173 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index dca2e49..975b62b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -88,6 +88,14 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
"hostdev",
"watchdog")
+VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
+ "none",
+ "pci")
+
+VIR_ENUM_IMPL(virDomainDeviceAddressMode, VIR_DOMAIN_DEVICE_ADDRESS_MODE_LAST,
+ "dynamic",
+ "static")
+
VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST,
"block",
"file",
@@ -729,6 +737,245 @@ void virDomainRemoveInactive(virDomainObjListPtr doms,
}
+int virDomainDeviceAddressIsValid(virDomainDeviceAddressPtr addr,
+ int type)
+{
+ if (addr->type != type)
+ return 0;
+
+ switch (addr->type) {
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
+ return virDomainDevicePCIAddressIsValid(&addr->data.pci);
+ }
+
+ return 0;
+}
+
+
+int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr)
+{
+ return addr->domain || addr->bus || addr->slot;
+}
+
+
+void virDomainDeviceAddressClear(virDomainDeviceAddressPtr addr)
+{
+ memset(addr, 0, sizeof(addr));
+ addr->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
+}
+
+
+/* Generate a string representation of a device address
+ * @param address Device address to stringify
+ */
+static int virDomainDeviceAddressFormat(virBufferPtr buf,
+ virDomainDeviceAddressPtr addr,
+ int flags)
+{
+ if (!addr) {
+ virDomainReportError(NULL, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing address information"));
+ return -1;
+ }
+
+ if (addr->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+ return 0;
+
+ /* Don't output dynamically allocated addresses when doing inactive XML dump */
+ if ((addr->mode == VIR_DOMAIN_DEVICE_ADDRESS_MODE_DYNAMIC) &&
+ (flags & VIR_DOMAIN_XML_INACTIVE))
+ return 0;
+
+ /* We'll be in domain/devices/[device type]/ so 3 level indent */
+ virBufferVSprintf(buf, " <address type='%s'",
+ virDomainDeviceAddressTypeToString(addr->type));
+ virBufferVSprintf(buf, " mode='%s'",
+ virDomainDeviceAddressModeTypeToString(addr->mode));
+
+ switch (addr->type) {
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
+ virBufferVSprintf(buf, " domain='0x%.4x' bus='0x%.2x'
slot='0x%.2x' function='0x%.1x'",
+ addr->data.pci.domain,
+ addr->data.pci.bus,
+ addr->data.pci.slot,
+ addr->data.pci.function);
+ break;
+
+ default:
+ virDomainReportError(NULL, VIR_ERR_INTERNAL_ERROR,
+ _("unknown address type '%d'"),
addr->type);
+ return -1;
+ }
+
+ virBufferAddLit(buf, "/>\n");
+
+ return 0;
+}
+
+/* Compare two device addresses. Returns true if addresses are
+ * identical and false if the they differ (or are of different types)
+ * @param a, @para b Pointers to the addresses
+ */
+int virDomainDeviceAddressEqual(virDomainDeviceAddressPtr a,
+ virDomainDeviceAddressPtr b)
+{
+ if (!a || !b || a->type != b-> type) {
+ return 0;
+ }
+
+ switch (a->type) {
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
+ return virDomainDevicePCIAddressEqual(&a->data.pci,
+ &b->data.pci);
+ }
+
+ return 0;
+}
+
+
+int virDomainDevicePCIAddressEqual(virDomainDevicePCIAddressPtr a,
+ virDomainDevicePCIAddressPtr b)
+{
+ if (a->domain == b->domain &&
+ a->bus == b->bus &&
+ a->slot == b->slot &&
+ a->function == b->function)
+ return 1;
+
+ return 0;
+}
+
+
+static int
+virDomainDevicePCIAddressParseXML(virConnectPtr conn,
+ xmlNodePtr node,
+ virDomainDevicePCIAddressPtr addr)
+{
+ char *domain, *slot, *bus, *function;
+ int ret = -1;
+
+ memset(addr, 0, sizeof(*addr));
+
+ domain = virXMLPropString(node, "domain");
+ bus = virXMLPropString(node, "bus");
+ slot = virXMLPropString(node, "slot");
+ function = virXMLPropString(node, "function");
+
+ if (domain &&
+ virStrToLong_ui(domain, NULL, 16, &addr->domain) < 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot parse <address> 'domain'
attribute"));
+ goto cleanup;
+ }
+
+ if (bus &&
+ virStrToLong_ui(bus, NULL, 16, &addr->bus) < 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot parse <address> 'bus'
attribute"));
+ goto cleanup;
+ }
+
+ if (slot &&
+ virStrToLong_ui(slot, NULL, 16, &addr->slot) < 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot parse <address> 'slot'
attribute"));
+ goto cleanup;
+ }
+
+ if (function &&
+ virStrToLong_ui(function, NULL, 16, &addr->function) < 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot parse <address> 'function'
attribute"));
+ goto cleanup;
+ }
+
+ if (!virDomainDevicePCIAddressIsValid(addr)) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Insufficient specification for PCI address"));
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(domain);
+ VIR_FREE(bus);
+ VIR_FREE(slot);
+ VIR_FREE(function);
+ return ret;
+}
+
+
+/* Parse the XML definition for a device address
+ * @param node XML nodeset to parse for device address definition
+ */
+static int
+virDomainDeviceAddressParseXML(virConnectPtr conn,
+ xmlNodePtr node,
+ virDomainDeviceAddressPtr addr,
+ int flags)
+{
+ char *type = NULL;
+ char *mode = NULL;
+ int ret = -1;
+
+ memset(addr, 0, sizeof(*addr));
+
+ type = virXMLPropString(node, "type");
+ mode = virXMLPropString(node, "mode");
+
+ if (type) {
+ if ((addr->type = virDomainDeviceAddressTypeFromString(type)) < 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("unknown address type '%s'"),
type);
+ goto cleanup;
+ }
+ } else {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("No type specified for device address"));
+ goto cleanup;
+ }
+
+ if (mode) {
+ if ((addr->mode = virDomainDeviceAddressModeTypeFromString(mode)) < 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("unknown address mode '%s'"),
mode);
+ goto cleanup;
+ }
+ } else {
+ addr->mode = VIR_DOMAIN_DEVICE_ADDRESS_MODE_DYNAMIC;
+ }
+
+ /* Dynamic addresses are only valid in the live config */
+ if ((flags & VIR_DOMAIN_XML_INACTIVE) &&
+ addr->mode == VIR_DOMAIN_DEVICE_ADDRESS_MODE_DYNAMIC) {
+ addr->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
+ ret = 0;
+ goto cleanup;
+ }
+
+ switch (addr->type) {
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
+ if (virDomainDevicePCIAddressParseXML(conn, node, &addr->data.pci) <
0)
+ goto cleanup;
+ break;
+
+ default:
+ /* Should not happen */
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("Unknown device address type"));
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(type);
+ VIR_FREE(mode);
+ return ret;
+}
+
+
/* Parse the XML definition for a disk
* @param node XML nodeset to parse for disk definition
*/
@@ -747,6 +994,7 @@ virDomainDiskDefParseXML(virConnectPtr conn,
char *bus = NULL;
char *cachetag = NULL;
char *devaddr = NULL;
+ xmlNodePtr addr = NULL;
virStorageEncryptionPtr encryption = NULL;
char *serial = NULL;
@@ -827,6 +1075,9 @@ virDomainDiskDefParseXML(virConnectPtr conn,
} else if ((serial == NULL) &&
(xmlStrEqual(cur->name, BAD_CAST "serial"))) {
serial = (char *)xmlNodeGetContent(cur);
+ } else if ((addr == NULL) &&
+ (xmlStrEqual(cur->name, BAD_CAST "address"))) {
+ addr = cur;
}
}
cur = cur->next;
@@ -926,15 +1177,20 @@ virDomainDiskDefParseXML(virConnectPtr conn,
goto error;
}
- if (devaddr &&
- sscanf(devaddr, "%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 devaddr parameter
'%s'"),
- devaddr);
- goto error;
+ if (devaddr) {
+ if (sscanf(devaddr, "%x:%x:%x",
+ &def->addr.data.pci.domain,
+ &def->addr.data.pci.bus,
+ &def->addr.data.pci.slot) < 3) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("Unable to parse devaddr parameter
'%s'"),
+ devaddr);
+ goto error;
+ }
+ def->addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+ } else if (addr) {
+ if (virDomainDeviceAddressParseXML(conn, addr, &def->addr, flags) < 0)
+ goto error;
}
def->src = source;
@@ -1083,6 +1339,7 @@ virDomainNetDefParseXML(virConnectPtr conn,
char *hostnet_name = NULL;
char *devaddr = NULL;
char *vlan = NULL;
+ xmlNodePtr addr = NULL;
if (VIR_ALLOC(def) < 0) {
virReportOOMError(conn);
@@ -1155,6 +1412,9 @@ virDomainNetDefParseXML(virConnectPtr conn,
hostnet_name = virXMLPropString(cur, "hostnet");
devaddr = virXMLPropString(cur, "devaddr");
vlan = virXMLPropString(cur, "vlan");
+ } else if ((addr == NULL) &&
+ (xmlStrEqual(cur->name, BAD_CAST "address"))) {
+ addr = cur;
}
}
cur = cur->next;
@@ -1171,14 +1431,28 @@ virDomainNetDefParseXML(virConnectPtr conn,
virCapabilitiesGenerateMac(caps, def->mac);
}
- if (devaddr &&
- sscanf(devaddr, "%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 devaddr parameter
'%s'"),
- devaddr);
+ if (devaddr) {
+ if (sscanf(devaddr, "%x:%x:%x",
+ &def->addr.data.pci.domain,
+ &def->addr.data.pci.bus,
+ &def->addr.data.pci.slot) < 3) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("Unable to parse devaddr parameter
'%s'"),
+ devaddr);
+ goto error;
+ }
+ def->addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+ } else if (addr) {
+ if (virDomainDeviceAddressParseXML(conn, addr, &def->addr, flags) < 0)
+ goto error;
+ }
+
+ /* XXX what about ISA/USB based NIC models - once we support
+ * them we should make sure address type is correct */
+ if (def->addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+ def->addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Network interfaces must use 'pci' address
type"));
goto error;
}
@@ -2319,83 +2593,26 @@ virDomainHostdevSubsysPciDefParseXML(virConnectPtr conn,
while (cur != NULL) {
if (cur->type == XML_ELEMENT_NODE) {
if (xmlStrEqual(cur->name, BAD_CAST "address")) {
+ virDomainDevicePCIAddressPtr addr =
+ &def->source.subsys.u.pci;
- char *domain = virXMLPropString(cur, "domain");
- if (domain) {
- if (virStrToLong_ui(domain, NULL, 0,
- &def->source.subsys.u.pci.domain) < 0) {
- virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
- _("cannot parse domain %s"),
- domain);
- VIR_FREE(domain);
- goto out;
- }
- VIR_FREE(domain);
- }
-
- char *bus = virXMLPropString(cur, "bus");
- if (bus) {
- if (virStrToLong_ui(bus, NULL, 0,
- &def->source.subsys.u.pci.bus) < 0) {
- virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
- _("cannot parse bus %s"), bus);
- VIR_FREE(bus);
- goto out;
- }
- VIR_FREE(bus);
- } else {
- virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
- "%s", _("pci address needs bus
id"));
- goto out;
- }
-
- char *slot = virXMLPropString(cur, "slot");
- if (slot) {
- if (virStrToLong_ui(slot, NULL, 0,
- &def->source.subsys.u.pci.slot) < 0)
{
- virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
- _("cannot parse slot %s"),
- slot);
- VIR_FREE(slot);
- goto out;
- }
- VIR_FREE(slot);
- } else {
- virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
- "%s", _("pci address needs slot
id"));
- goto out;
- }
-
- char *function = virXMLPropString(cur, "function");
- if (function) {
- if (virStrToLong_ui(function, NULL, 0,
- &def->source.subsys.u.pci.function) < 0)
{
- virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
- _("cannot parse function %s"),
- function);
- VIR_FREE(function);
- goto out;
- }
- VIR_FREE(function);
- } else {
- virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
- _("pci address needs function id"));
+ if (virDomainDevicePCIAddressParseXML(conn, cur, addr) < 0)
goto out;
- }
} else if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) &&
xmlStrEqual(cur->name, BAD_CAST "state")) {
char *devaddr = virXMLPropString(cur, "devaddr");
if (devaddr &&
sscanf(devaddr, "%x:%x:%x",
- &def->source.subsys.u.pci.guest_addr.domain,
- &def->source.subsys.u.pci.guest_addr.bus,
- &def->source.subsys.u.pci.guest_addr.slot) < 3) {
+ &def->addr.data.pci.domain,
+ &def->addr.data.pci.bus,
+ &def->addr.data.pci.slot) < 3) {
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("Unable to parse devaddr parameter
'%s'"),
devaddr);
VIR_FREE(devaddr);
goto out;
}
+ def->addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
} else {
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("unknown pci source type
'%s'"),
@@ -2473,6 +2690,9 @@ virDomainHostdevDefParseXML(virConnectPtr conn,
if (virDomainHostdevSubsysPciDefParseXML(conn, cur, def, flags)
< 0)
goto error;
}
+ } else if (xmlStrEqual(cur->name, BAD_CAST "address")) {
+ if (virDomainDeviceAddressParseXML(conn, cur, &def->addr, flags)
< 0)
+ goto error;
} else {
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("unknown node %s"), cur->name);
@@ -2481,6 +2701,18 @@ virDomainHostdevDefParseXML(virConnectPtr conn,
cur = cur->next;
}
+ if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
+ switch (def->source.subsys.type) {
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
+ if (def->addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("PCI host devices must use 'pci'
address type"));
+ goto error;
+ }
+ break;
+ }
+ }
+
cleanup:
VIR_FREE(type);
VIR_FREE(mode);
@@ -3934,15 +4166,9 @@ virDomainDiskDefFormat(virConnectPtr conn,
virStorageEncryptionFormat(conn, buf, def->encryption) < 0)
return -1;
- if (flags & VIR_DOMAIN_XML_INTERNAL_STATUS) {
- virBufferAddLit(buf, " <state");
- if (virDiskHasValidPciAddr(def))
- virBufferVSprintf(buf, " devaddr='%.4x:%.2x:%.2x'",
- def->pci_addr.domain,
- def->pci_addr.bus,
- def->pci_addr.slot);
- virBufferAddLit(buf, "/>\n");
- }
+ if (def->addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+ virDomainDeviceAddressFormat(buf, &def->addr, flags) < 0)
+ return -1;
virBufferAddLit(buf, " </disk>\n");
@@ -4081,16 +4307,15 @@ virDomainNetDefFormat(virConnectPtr conn,
virBufferEscapeString(buf, " nic='%s'", def->nic_name);
if (def->hostnet_name)
virBufferEscapeString(buf, " hostnet='%s'",
def->hostnet_name);
- if (virNetHasValidPciAddr(def))
- virBufferVSprintf(buf, " devaddr='%.4x:%.2x:%.2x'",
- def->pci_addr.domain,
- def->pci_addr.bus,
- def->pci_addr.slot);
if (def->vlan > 0)
virBufferVSprintf(buf, " vlan='%d'", def->vlan);
virBufferAddLit(buf, "/>\n");
}
+ if (def->addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+ virDomainDeviceAddressFormat(buf, &def->addr, flags) < 0)
+ return -1;
+
virBufferAddLit(buf, " </interface>\n");
return 0;
@@ -4499,25 +4724,20 @@ virDomainHostdevDefFormat(virConnectPtr conn,
def->source.subsys.u.usb.bus,
def->source.subsys.u.usb.device);
}
- }
- if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
+ } else if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
virBufferVSprintf(buf, " <address domain='0x%.4x'
bus='0x%.2x' slot='0x%.2x' function='0x%.1x'/>\n",
def->source.subsys.u.pci.domain,
def->source.subsys.u.pci.bus,
def->source.subsys.u.pci.slot,
def->source.subsys.u.pci.function);
- if (flags & VIR_DOMAIN_XML_INTERNAL_STATUS) {
- virBufferAddLit(buf, " <state");
- if (virHostdevHasValidGuestAddr(def))
- virBufferVSprintf(buf, " devaddr='%.4x:%.2x:%.2x'",
- def->source.subsys.u.pci.guest_addr.domain,
- def->source.subsys.u.pci.guest_addr.bus,
- def->source.subsys.u.pci.guest_addr.slot);
- virBufferAddLit(buf, "/>\n");
- }
}
virBufferAddLit(buf, " </source>\n");
+
+ if (def->addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+ virDomainDeviceAddressFormat(buf, &def->addr, flags) < 0)
+ return -1;
+
virBufferAddLit(buf, " </hostdev>\n");
return 0;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ac39dcd..31a2f9d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -63,6 +63,40 @@ enum virDomainVirtType {
VIR_DOMAIN_VIRT_LAST,
};
+enum virDomainDeviceAddressType {
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE,
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI,
+
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST
+};
+
+enum virDomainDeviceAddressMode {
+ VIR_DOMAIN_DEVICE_ADDRESS_MODE_DYNAMIC,
+ VIR_DOMAIN_DEVICE_ADDRESS_MODE_STATIC,
+
+ VIR_DOMAIN_DEVICE_ADDRESS_MODE_LAST
+};
+
+typedef struct _virDomainDevicePCIAddress virDomainDevicePCIAddress;
+typedef virDomainDevicePCIAddress *virDomainDevicePCIAddressPtr;
+struct _virDomainDevicePCIAddress {
+ unsigned int domain;
+ unsigned int bus;
+ unsigned int slot;
+ unsigned int function;
+};
+
+typedef struct _virDomainDeviceAddress virDomainDeviceAddress;
+typedef virDomainDeviceAddress *virDomainDeviceAddressPtr;
+struct _virDomainDeviceAddress {
+ int type;
+ int mode;
+ union {
+ virDomainDevicePCIAddress pci;
+ } data;
+};
+
+
/* Two types of disk backends */
enum virDomainDiskType {
VIR_DOMAIN_DISK_TYPE_BLOCK,
@@ -118,20 +152,10 @@ struct _virDomainDiskDef {
int cachemode;
unsigned int readonly : 1;
unsigned int shared : 1;
- struct {
- unsigned domain;
- unsigned bus;
- unsigned slot;
- } pci_addr;
+ virDomainDeviceAddress addr;
virStorageEncryptionPtr encryption;
};
-static inline int
-virDiskHasValidPciAddr(virDomainDiskDefPtr def)
-{
- return def->pci_addr.domain || def->pci_addr.bus || def->pci_addr.slot;
-}
-
/* Two types of disk backends */
enum virDomainFSType {
@@ -200,20 +224,10 @@ struct _virDomainNetDef {
char *ifname;
char *nic_name;
char *hostnet_name;
- struct {
- unsigned domain;
- unsigned bus;
- unsigned slot;
- } pci_addr;
+ virDomainDeviceAddress addr;
int vlan;
};
-static inline int
-virNetHasValidPciAddr(virDomainNetDefPtr def)
-{
- return def->pci_addr.domain || def->pci_addr.bus || def->pci_addr.slot;
-}
-
enum virDomainChrTargetType {
VIR_DOMAIN_CHR_TARGET_TYPE_NULL = 0,
VIR_DOMAIN_CHR_TARGET_TYPE_MONITOR,
@@ -441,17 +455,7 @@ struct _virDomainHostdevDef {
unsigned vendor;
unsigned product;
} usb;
- struct {
- unsigned domain;
- unsigned bus;
- unsigned slot;
- unsigned function;
- struct {
- unsigned domain;
- unsigned bus;
- unsigned slot;
- } guest_addr;
- } pci;
+ virDomainDevicePCIAddress pci; /* host address */
} u;
} subsys;
struct {
@@ -462,16 +466,9 @@ struct _virDomainHostdevDef {
} caps;
} source;
char* target;
+ virDomainDeviceAddress addr; /* Guest address */
};
-static inline int
-virHostdevHasValidGuestAddr(virDomainHostdevDefPtr def)
-{
- return def->source.subsys.u.pci.guest_addr.domain ||
- def->source.subsys.u.pci.guest_addr.bus ||
- def->source.subsys.u.pci.guest_addr.slot;
-}
-
/* Flags for the 'type' field in next struct */
enum virDomainDeviceType {
VIR_DOMAIN_DEVICE_DISK,
@@ -671,6 +668,7 @@ virDomainObjIsActive(virDomainObjPtr dom)
return dom->def->id != -1;
}
+
int virDomainObjListInit(virDomainObjListPtr objs);
void virDomainObjListDeinit(virDomainObjListPtr objs);
@@ -693,6 +691,14 @@ void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def);
void virDomainVideoDefFree(virDomainVideoDefPtr def);
void virDomainHostdevDefFree(virDomainHostdevDefPtr def);
void virDomainDeviceDefFree(virDomainDeviceDefPtr def);
+int virDomainDeviceAddressEqual(virDomainDeviceAddressPtr a,
+ virDomainDeviceAddressPtr b);
+int virDomainDevicePCIAddressEqual(virDomainDevicePCIAddressPtr a,
+ virDomainDevicePCIAddressPtr b);
+int virDomainDeviceAddressIsValid(virDomainDeviceAddressPtr addr,
+ int type);
+int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr);
+void virDomainDeviceAddressClear(virDomainDeviceAddressPtr addr);
void virDomainDefFree(virDomainDefPtr vm);
void virDomainObjRef(virDomainObjPtr vm);
/* Returns 1 if the object was freed, 0 if more refs exist */
@@ -830,6 +836,8 @@ VIR_ENUM_DECL(virDomainBoot)
VIR_ENUM_DECL(virDomainFeature)
VIR_ENUM_DECL(virDomainLifecycle)
VIR_ENUM_DECL(virDomainDevice)
+VIR_ENUM_DECL(virDomainDeviceAddress)
+VIR_ENUM_DECL(virDomainDeviceAddressMode)
VIR_ENUM_DECL(virDomainDisk)
VIR_ENUM_DECL(virDomainDiskDevice)
VIR_ENUM_DECL(virDomainDiskBus)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 72257d7..963206b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -154,6 +154,11 @@ virDomainObjListInit;
virDomainObjListDeinit;
virDomainObjRef;
virDomainObjUnref;
+virDomainDeviceAddressEqual;
+virDomainDevicePCIAddressEqual;
+virDomainDeviceAddressIsValid;
+virDomainDevicePCIAddressIsValid;
+virDomainDeviceAddressClear;
# domain_event.h
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7e60d0e..5920ab3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4947,13 +4947,15 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
ret = qemuMonitorAddPCIDisk(priv->mon,
dev->data.disk->src,
type,
- &dev->data.disk->pci_addr.domain,
- &dev->data.disk->pci_addr.bus,
- &dev->data.disk->pci_addr.slot);
+ &dev->data.disk->addr.data.pci.domain,
+ &dev->data.disk->addr.data.pci.bus,
+ &dev->data.disk->addr.data.pci.slot);
qemuDomainObjExitMonitorWithDriver(driver, vm);
- if (ret == 0)
+ if (ret == 0) {
+ dev->data.disk->addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
virDomainDiskInsertPreAlloced(vm->def, dev->data.disk);
+ }
return ret;
}
@@ -5076,12 +5078,13 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn,
qemuDomainObjEnterMonitorWithDriver(driver, vm);
if (qemuMonitorAddPCINetwork(priv->mon, nicstr,
- &net->pci_addr.domain,
- &net->pci_addr.bus,
- &net->pci_addr.slot) < 0) {
+ &net->addr.data.pci.domain,
+ &net->addr.data.pci.bus,
+ &net->addr.data.pci.slot) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
goto try_remove;
}
+ net->addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
qemuDomainObjExitMonitorWithDriver(driver, vm);
ret = 0;
@@ -5164,12 +5167,13 @@ static int qemudDomainAttachHostPciDevice(virConnectPtr conn,
hostdev->source.subsys.u.pci.bus,
hostdev->source.subsys.u.pci.slot,
hostdev->source.subsys.u.pci.function,
-
&hostdev->source.subsys.u.pci.guest_addr.domain,
-
&hostdev->source.subsys.u.pci.guest_addr.bus,
-
&hostdev->source.subsys.u.pci.guest_addr.slot);
+ &hostdev->addr.data.pci.domain,
+ &hostdev->addr.data.pci.bus,
+ &hostdev->addr.data.pci.slot);
qemuDomainObjExitMonitorWithDriver(driver, vm);
if (ret < 0)
goto error;
+ hostdev->addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
@@ -5399,18 +5403,18 @@ static int qemudDomainDetachPciDiskDevice(virConnectPtr conn,
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);
+ if (!virDomainDeviceAddressIsValid(&detach->addr,
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, "%s",
+ _("device cannot be detached without a PCI
address"));
goto cleanup;
}
qemuDomainObjEnterMonitorWithDriver(driver, vm);
if (qemuMonitorRemovePCIDevice(priv->mon,
- detach->pci_addr.domain,
- detach->pci_addr.bus,
- detach->pci_addr.slot) < 0) {
+ detach->addr.data.pci.domain,
+ detach->addr.data.pci.bus,
+ detach->addr.data.pci.slot) < 0) {
qemuDomainObjExitMonitor(vm);
goto cleanup;
}
@@ -5465,7 +5469,14 @@ qemudDomainDetachNetDevice(virConnectPtr conn,
goto cleanup;
}
- if (!virNetHasValidPciAddr(detach) || detach->vlan < 0 ||
!detach->hostnet_name) {
+ if (!virDomainDeviceAddressIsValid(&detach->addr,
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ "%s", _("device cannot be detached without a PCI
address"));
+ goto cleanup;
+ }
+
+ if (detach->vlan < 0 || !detach->hostnet_name) {
qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
"%s", _("network device cannot be detached -
device state missing"));
goto cleanup;
@@ -5473,9 +5484,9 @@ qemudDomainDetachNetDevice(virConnectPtr conn,
qemuDomainObjEnterMonitorWithDriver(driver, vm);
if (qemuMonitorRemovePCIDevice(priv->mon,
- detach->pci_addr.domain,
- detach->pci_addr.bus,
- detach->pci_addr.slot) < 0) {
+ detach->addr.data.pci.domain,
+ detach->addr.data.pci.bus,
+ detach->addr.data.pci.slot) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
goto cleanup;
}
@@ -5553,17 +5564,18 @@ static int qemudDomainDetachHostPciDevice(virConnectPtr conn,
return -1;
}
- if (!virHostdevHasValidGuestAddr(detach)) {
+ if (!virDomainDeviceAddressIsValid(&detach->addr,
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
- "%s", _("hostdev cannot be detached - device
state missing"));
+ "%s", _("device cannot be detached without a PCI
address"));
return -1;
}
qemuDomainObjEnterMonitorWithDriver(driver, vm);
if (qemuMonitorRemovePCIDevice(priv->mon,
- detach->source.subsys.u.pci.guest_addr.domain,
- detach->source.subsys.u.pci.guest_addr.bus,
- detach->source.subsys.u.pci.guest_addr.slot) <
0) {
+ detach->addr.data.pci.domain,
+ detach->addr.data.pci.bus,
+ detach->addr.data.pci.slot) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
return -1;
}
--
1.6.5.2
4:22 p.m.
New subject: [libvirt] [PATCH 02/12] Extend the virDomainDeviceAddress struture to allow USB addresses
Introduce a new structure
struct _virDomainDeviceUSBAddress {
unsigned int bus;
unsigned int dev;
};
and plug that into virDomainDeviceAddress. Convert the
host device USB config to use this new address struct.
XML looks like
<address type='usb' bus='007' dev='003'/>
---
src/conf/domain_conf.c | 93 ++++++++++++++++++++++++++++++++++++--
src/conf/domain_conf.h | 15 +++++-
src/libvirt_private.syms | 2 +
src/qemu/qemu_conf.c | 12 +++---
src/qemu/qemu_driver.c | 13 +++---
src/security/security_selinux.c | 26 ++++++-----
6 files changed, 130 insertions(+), 31 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 975b62b..0e88362 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -90,7 +90,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
"none",
- "pci")
+ "pci",
+ "usb");
VIR_ENUM_IMPL(virDomainDeviceAddressMode, VIR_DOMAIN_DEVICE_ADDRESS_MODE_LAST,
"dynamic",
@@ -746,6 +747,9 @@ int virDomainDeviceAddressIsValid(virDomainDeviceAddressPtr addr,
switch (addr->type) {
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
return virDomainDevicePCIAddressIsValid(&addr->data.pci);
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
+ return virDomainDeviceUSBAddressIsValid(&addr->data.usb);
}
return 0;
@@ -758,6 +762,12 @@ int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr
addr)
}
+int virDomainDeviceUSBAddressIsValid(virDomainDeviceUSBAddressPtr addr)
+{
+ return addr->bus || addr->dev;
+}
+
+
void virDomainDeviceAddressClear(virDomainDeviceAddressPtr addr)
{
memset(addr, 0, sizeof(addr));
@@ -801,6 +811,12 @@ static int virDomainDeviceAddressFormat(virBufferPtr buf,
addr->data.pci.function);
break;
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
+ virBufferVSprintf(buf, " bus='%.3d' dev='%.3d'",
+ addr->data.usb.bus,
+ addr->data.usb.dev);
+ break;
+
default:
virDomainReportError(NULL, VIR_ERR_INTERNAL_ERROR,
_("unknown address type '%d'"),
addr->type);
@@ -827,6 +843,9 @@ int virDomainDeviceAddressEqual(virDomainDeviceAddressPtr a,
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
return virDomainDevicePCIAddressEqual(&a->data.pci,
&b->data.pci);
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
+ return virDomainDeviceUSBAddressEqual(&a->data.usb,
+ &b->data.usb);
}
return 0;
@@ -846,6 +865,17 @@ int virDomainDevicePCIAddressEqual(virDomainDevicePCIAddressPtr a,
}
+int virDomainDeviceUSBAddressEqual(virDomainDeviceUSBAddressPtr a,
+ virDomainDeviceUSBAddressPtr b)
+{
+ if (a->bus == b->bus &&
+ a->dev == b->dev)
+ return 1;
+
+ return 0;
+}
+
+
static int
virDomainDevicePCIAddressParseXML(virConnectPtr conn,
xmlNodePtr node,
@@ -905,6 +935,47 @@ cleanup:
return ret;
}
+static int
+virDomainDeviceUSBAddressParseXML(virConnectPtr conn,
+ xmlNodePtr node,
+ virDomainDeviceUSBAddressPtr addr)
+{
+ char *bus, *dev;
+ int ret = -1;
+
+ memset(addr, 0, sizeof(*addr));
+
+ bus = virXMLPropString(node, "bus");
+ dev = virXMLPropString(node, "dev");
+
+ if (bus &&
+ virStrToLong_ui(bus, NULL, 10, &addr->bus) < 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot parse <address> 'bus'
attribute"));
+ goto cleanup;
+ }
+
+ if (dev &&
+ virStrToLong_ui(dev, NULL, 10, &addr->dev) < 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot parse <address> 'dev'
attribute"));
+ goto cleanup;
+ }
+
+ if (!virDomainDeviceUSBAddressIsValid(addr)) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Insufficient specification for USB address"));
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(bus);
+ VIR_FREE(dev);
+ return ret;
+}
+
/* Parse the XML definition for a device address
* @param node XML nodeset to parse for device address definition
@@ -960,6 +1031,11 @@ virDomainDeviceAddressParseXML(virConnectPtr conn,
goto cleanup;
break;
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
+ if (virDomainDeviceUSBAddressParseXML(conn, node, &addr->data.usb) <
0)
+ goto cleanup;
+ break;
+
default:
/* Should not happen */
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
@@ -2518,7 +2594,7 @@ virDomainHostdevSubsysUsbDefParseXML(virConnectPtr conn,
bus = virXMLPropString(cur, "bus");
if (bus) {
if (virStrToLong_ui(bus, NULL, 0,
- &def->source.subsys.u.usb.bus) < 0) {
+ &def->source.subsys.u.usb.addr.bus) <
0) {
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("cannot parse bus %s"), bus);
VIR_FREE(bus);
@@ -2534,7 +2610,7 @@ virDomainHostdevSubsysUsbDefParseXML(virConnectPtr conn,
device = virXMLPropString(cur, "device");
if (device) {
if (virStrToLong_ui(device, NULL, 0,
- &def->source.subsys.u.usb.device) < 0)
{
+ &def->source.subsys.u.usb.addr.dev) <
0) {
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("cannot parse device %s"),
device);
@@ -2710,6 +2786,13 @@ virDomainHostdevDefParseXML(virConnectPtr conn,
goto error;
}
break;
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
+ if (def->addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("USB host devices must use 'usb'
address type"));
+ goto error;
+ }
+ break;
}
}
@@ -4721,8 +4804,8 @@ virDomainHostdevDefFormat(virConnectPtr conn,
def->source.subsys.u.usb.product);
} else {
virBufferVSprintf(buf, " <address bus='%d'
device='%d'/>\n",
- def->source.subsys.u.usb.bus,
- def->source.subsys.u.usb.device);
+ def->source.subsys.u.usb.addr.bus,
+ def->source.subsys.u.usb.addr.dev);
}
} else if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
virBufferVSprintf(buf, " <address domain='0x%.4x'
bus='0x%.2x' slot='0x%.2x' function='0x%.1x'/>\n",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 31a2f9d..1ae63bd 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -66,6 +66,7 @@ enum virDomainVirtType {
enum virDomainDeviceAddressType {
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI,
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST
};
@@ -86,6 +87,13 @@ struct _virDomainDevicePCIAddress {
unsigned int function;
};
+typedef struct _virDomainDeviceUSBAddress virDomainDeviceUSBAddress;
+typedef virDomainDeviceUSBAddress *virDomainDeviceUSBAddressPtr;
+struct _virDomainDeviceUSBAddress {
+ unsigned int bus;
+ unsigned int dev;
+};
+
typedef struct _virDomainDeviceAddress virDomainDeviceAddress;
typedef virDomainDeviceAddress *virDomainDeviceAddressPtr;
struct _virDomainDeviceAddress {
@@ -93,6 +101,7 @@ struct _virDomainDeviceAddress {
int mode;
union {
virDomainDevicePCIAddress pci;
+ virDomainDeviceUSBAddress usb;
} data;
};
@@ -449,8 +458,7 @@ struct _virDomainHostdevDef {
int type; /* enum virDomainHostdevBusType */
union {
struct {
- unsigned bus;
- unsigned device;
+ virDomainDeviceUSBAddress addr;
unsigned vendor;
unsigned product;
@@ -695,9 +703,12 @@ int virDomainDeviceAddressEqual(virDomainDeviceAddressPtr a,
virDomainDeviceAddressPtr b);
int virDomainDevicePCIAddressEqual(virDomainDevicePCIAddressPtr a,
virDomainDevicePCIAddressPtr b);
+int virDomainDeviceUSBAddressEqual(virDomainDeviceUSBAddressPtr a,
+ virDomainDeviceUSBAddressPtr b);
int virDomainDeviceAddressIsValid(virDomainDeviceAddressPtr addr,
int type);
int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr);
+int virDomainDeviceUSBAddressIsValid(virDomainDeviceUSBAddressPtr addr);
void virDomainDeviceAddressClear(virDomainDeviceAddressPtr addr);
void virDomainDefFree(virDomainDefPtr vm);
void virDomainObjRef(virDomainObjPtr vm);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 963206b..49df15c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -156,8 +156,10 @@ virDomainObjRef;
virDomainObjUnref;
virDomainDeviceAddressEqual;
virDomainDevicePCIAddressEqual;
+virDomainDeviceUSBAddressEqual;
virDomainDeviceAddressIsValid;
virDomainDevicePCIAddressIsValid;
+virDomainDeviceUSBAddressIsValid;
virDomainDeviceAddressClear;
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 7d41b5d..39d9314 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -2466,15 +2466,15 @@ int qemudBuildCommandLine(virConnectPtr conn,
/* USB */
if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
- if(hostdev->source.subsys.u.usb.vendor) {
- ret = virAsprintf(&usbdev, "host:%.4x:%.4x",
+ if
(!virDomainDeviceUSBAddressIsValid(&hostdev->source.subsys.u.usb.addr)) {
+ ret = virAsprintf(&usbdev, "host:%.4x:%.4x",
hostdev->source.subsys.u.usb.vendor,
hostdev->source.subsys.u.usb.product);
} else {
ret = virAsprintf(&usbdev, "host:%.3d.%.3d",
- hostdev->source.subsys.u.usb.bus,
- hostdev->source.subsys.u.usb.device);
+ hostdev->source.subsys.u.usb.addr.bus,
+ hostdev->source.subsys.u.usb.addr.dev);
}
if (ret < 0)
goto error;
@@ -3189,8 +3189,8 @@ qemuParseCommandLineUSB(virConnectPtr conn,
def->managed = 0;
def->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB;
if (*end == '.') {
- def->source.subsys.u.usb.bus = first;
- def->source.subsys.u.usb.device = second;
+ def->source.subsys.u.usb.addr.bus = first;
+ def->source.subsys.u.usb.addr.dev = second;
} else {
def->source.subsys.u.usb.vendor = first;
def->source.subsys.u.usb.product = second;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5920ab3..da4fae7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2019,13 +2019,12 @@ static int qemuDomainSetHostdevUSBOwnership(virConnectPtr conn,
int ret = -1;
/* XXX what todo for USB devs assigned based on product/vendor ? Doom :-( */
- if (!def->source.subsys.u.usb.bus ||
- !def->source.subsys.u.usb.device)
+ if (!virDomainDeviceUSBAddressIsValid(&def->source.subsys.u.usb.addr))
return 0;
usbDevice *dev = usbGetDevice(conn,
- def->source.subsys.u.usb.bus,
- def->source.subsys.u.usb.device);
+ def->source.subsys.u.usb.addr.bus,
+ def->source.subsys.u.usb.addr.dev);
if (!dev)
goto cleanup;
@@ -5199,14 +5198,14 @@ static int qemudDomainAttachHostUsbDevice(virConnectPtr conn,
}
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- if (dev->data.hostdev->source.subsys.u.usb.vendor) {
+ if
(!virDomainDeviceUSBAddressIsValid(&dev->data.hostdev->source.subsys.u.usb.addr))
{
ret = qemuMonitorAddUSBDeviceMatch(priv->mon,
dev->data.hostdev->source.subsys.u.usb.vendor,
dev->data.hostdev->source.subsys.u.usb.product);
} else {
ret = qemuMonitorAddUSBDeviceExact(priv->mon,
-
dev->data.hostdev->source.subsys.u.usb.bus,
-
dev->data.hostdev->source.subsys.u.usb.device);
+
dev->data.hostdev->source.subsys.u.usb.addr.bus,
+
dev->data.hostdev->source.subsys.u.usb.addr.dev);
}
qemuDomainObjExitMonitorWithDriver(driver, vm);
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 255ba53..1710651 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -481,10 +481,10 @@ SELinuxSetSecurityHostdevLabel(virConnectPtr conn,
switch (dev->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
- if (dev->source.subsys.u.usb.bus &&
dev->source.subsys.u.usb.device) {
+ if (virDomainDeviceUSBAddressIsValid(&dev->source.subsys.u.usb.addr)) {
usbDevice *usb = usbGetDevice(conn,
- dev->source.subsys.u.usb.bus,
- dev->source.subsys.u.usb.device);
+ dev->source.subsys.u.usb.addr.bus,
+ dev->source.subsys.u.usb.addr.dev);
if (!usb)
goto done;
@@ -554,16 +554,20 @@ SELinuxRestoreSecurityHostdevLabel(virConnectPtr conn,
switch (dev->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
- usbDevice *usb = usbGetDevice(conn,
- dev->source.subsys.u.usb.bus,
- dev->source.subsys.u.usb.device);
-
- if (!usb)
- goto done;
+ if (virDomainDeviceUSBAddressIsValid(&dev->source.subsys.u.usb.addr)) {
+ usbDevice *usb = usbGetDevice(conn,
+ dev->source.subsys.u.usb.addr.bus,
+ dev->source.subsys.u.usb.addr.dev);
- ret = usbDeviceFileIterate(conn, usb, SELinuxRestoreSecurityUSBLabel, NULL);
- usbFreeDevice(conn, usb);
+ if (!usb)
+ goto done;
+ ret = usbDeviceFileIterate(conn, usb, SELinuxRestoreSecurityUSBLabel, NULL);
+ usbFreeDevice(conn, usb);
+ } else {
+ /* XXX deal with product/vendor better */
+ ret = 0;
+ }
break;
}
--
1.6.5.2
3:32 a.m.
New subject: [libvirt] Re: [PATCH 02/12] Extend the virDomainDeviceAddress struture to allow USB addresses
On Thu, Dec 10, 2009 at 10:22:22PM +0000, Daniel P. Berrange wrote:
Introduce a new structure
struct _virDomainDeviceUSBAddress {
unsigned int bus;
unsigned int dev;
};
and plug that into virDomainDeviceAddress. Convert the
host device USB config to use this new address struct.
XML looks like
<address type='usb' bus='007' dev='003'/>
I think I will remove this particular patch. After investigating a
little more, it appears that 'dev' is not something you can ever
assign from the host. The guest OS assigns 'dev' on the fly when
it activates the USB device. We might be able to assign 'bus' from
the host, but QEMU doesn't currently allow it. We can always re-add
this patch later when needed.
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 :|
4:22 p.m.
New subject: [libvirt] [PATCH 03/12] Extend the virDomainDeviceAddress struture to allow disk controller addresses
Introduce a new structure
struct _virDomainDeviceDriveAddress {
unsigned int controller;
unsigned int bus;
unsigned int unit;
};
and plug that into virDomainDeviceAddress and generates XML that
looks like
<address type='drive' controller='1' bus='0'
unit='5'/>
* src/conf/domain_conf.h, src/conf/domain_conf.c: Parsing and
formatting of drive addresses
---
src/conf/domain_conf.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++-
src/conf/domain_conf.h | 13 +++++++
2 files changed, 100 insertions(+), 1 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0e88362..9c0b8cf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -91,7 +91,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
"none",
"pci",
- "usb");
+ "usb",
+ "drive");
VIR_ENUM_IMPL(virDomainDeviceAddressMode, VIR_DOMAIN_DEVICE_ADDRESS_MODE_LAST,
"dynamic",
@@ -750,6 +751,9 @@ int virDomainDeviceAddressIsValid(virDomainDeviceAddressPtr addr,
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
return virDomainDeviceUSBAddressIsValid(&addr->data.usb);
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
+ return virDomainDeviceDriveAddressIsValid(&addr->data.drive);
}
return 0;
@@ -767,6 +771,11 @@ int virDomainDeviceUSBAddressIsValid(virDomainDeviceUSBAddressPtr
addr)
return addr->bus || addr->dev;
}
+int virDomainDeviceDriveAddressIsValid(virDomainDeviceDriveAddressPtr addr
ATTRIBUTE_UNUSED)
+{
+ /*return addr->controller || addr->bus || addr->unit;*/
+ return 1; /* 0 is valid for all fields, so any successfully parsed addr is valid */
+}
void virDomainDeviceAddressClear(virDomainDeviceAddressPtr addr)
{
@@ -817,6 +826,13 @@ static int virDomainDeviceAddressFormat(virBufferPtr buf,
addr->data.usb.dev);
break;
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
+ virBufferVSprintf(buf, " controller='%d' bus='%d'
unit='%d'",
+ addr->data.drive.controller,
+ addr->data.drive.bus,
+ addr->data.drive.unit);
+ break;
+
default:
virDomainReportError(NULL, VIR_ERR_INTERNAL_ERROR,
_("unknown address type '%d'"),
addr->type);
@@ -846,6 +862,9 @@ int virDomainDeviceAddressEqual(virDomainDeviceAddressPtr a,
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
return virDomainDeviceUSBAddressEqual(&a->data.usb,
&b->data.usb);
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
+ return virDomainDeviceDriveAddressEqual(&a->data.drive,
+ &b->data.drive);
}
return 0;
@@ -876,6 +895,18 @@ int virDomainDeviceUSBAddressEqual(virDomainDeviceUSBAddressPtr a,
}
+int virDomainDeviceDriveAddressEqual(virDomainDeviceDriveAddressPtr a,
+ virDomainDeviceDriveAddressPtr b)
+{
+ if (a->controller == b->controller &&
+ a->bus == b->bus &&
+ a->unit == b->unit)
+ return 1;
+
+ return 0;
+}
+
+
static int
virDomainDevicePCIAddressParseXML(virConnectPtr conn,
xmlNodePtr node,
@@ -977,6 +1008,56 @@ cleanup:
}
+static int
+virDomainDeviceDriveAddressParseXML(virConnectPtr conn,
+ xmlNodePtr node,
+ virDomainDeviceDriveAddressPtr addr)
+{
+ char *bus, *unit, *controller;
+ int ret = -1;
+
+ memset(addr, 0, sizeof(*addr));
+
+ controller = virXMLPropString(node, "controller");
+ bus = virXMLPropString(node, "bus");
+ unit = virXMLPropString(node, "unit");
+
+ if (controller &&
+ virStrToLong_ui(controller, NULL, 10, &addr->controller) < 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot parse <address> 'controller'
attribute"));
+ goto cleanup;
+ }
+
+ if (bus &&
+ virStrToLong_ui(bus, NULL, 10, &addr->bus) < 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot parse <address> 'bus'
attribute"));
+ goto cleanup;
+ }
+
+ if (unit &&
+ virStrToLong_ui(unit, NULL, 10, &addr->unit) < 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot parse <address> 'unit'
attribute"));
+ goto cleanup;
+ }
+
+ if (!virDomainDeviceDriveAddressIsValid(addr)) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Insufficient specification for drive
address"));
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(controller);
+ VIR_FREE(bus);
+ VIR_FREE(unit);
+ return ret;
+}
+
/* Parse the XML definition for a device address
* @param node XML nodeset to parse for device address definition
*/
@@ -1036,6 +1117,11 @@ virDomainDeviceAddressParseXML(virConnectPtr conn,
goto cleanup;
break;
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
+ if (virDomainDeviceDriveAddressParseXML(conn, node, &addr->data.drive)
< 0)
+ goto cleanup;
+ break;
+
default:
/* Should not happen */
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1ae63bd..d1c1b67 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -67,6 +67,7 @@ enum virDomainDeviceAddressType {
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB,
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST
};
@@ -94,6 +95,14 @@ struct _virDomainDeviceUSBAddress {
unsigned int dev;
};
+typedef struct _virDomainDeviceDriveAddress virDomainDeviceDriveAddress;
+typedef virDomainDeviceDriveAddress *virDomainDeviceDriveAddressPtr;
+struct _virDomainDeviceDriveAddress {
+ unsigned int controller;
+ unsigned int bus;
+ unsigned int unit;
+};
+
typedef struct _virDomainDeviceAddress virDomainDeviceAddress;
typedef virDomainDeviceAddress *virDomainDeviceAddressPtr;
struct _virDomainDeviceAddress {
@@ -102,6 +111,7 @@ struct _virDomainDeviceAddress {
union {
virDomainDevicePCIAddress pci;
virDomainDeviceUSBAddress usb;
+ virDomainDeviceDriveAddress drive;
} data;
};
@@ -705,10 +715,13 @@ int virDomainDevicePCIAddressEqual(virDomainDevicePCIAddressPtr a,
virDomainDevicePCIAddressPtr b);
int virDomainDeviceUSBAddressEqual(virDomainDeviceUSBAddressPtr a,
virDomainDeviceUSBAddressPtr b);
+int virDomainDeviceDriveAddressEqual(virDomainDeviceDriveAddressPtr a,
+ virDomainDeviceDriveAddressPtr b);
int virDomainDeviceAddressIsValid(virDomainDeviceAddressPtr addr,
int type);
int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr);
int virDomainDeviceUSBAddressIsValid(virDomainDeviceUSBAddressPtr addr);
+int virDomainDeviceDriveAddressIsValid(virDomainDeviceDriveAddressPtr addr);
void virDomainDeviceAddressClear(virDomainDeviceAddressPtr addr);
void virDomainDefFree(virDomainDefPtr vm);
void virDomainObjRef(virDomainObjPtr vm);
--
1.6.5.2
4:22 p.m.
New subject: [libvirt] [PATCH 04/12] Add address info to sound, video and watchdog devices
Add the virDomainDeviceAddress information to the sound, video
and watchdog devices. This means all of them gain a new XML
element
<address .... />
* src/conf/domain_conf.h: Add virDomainDeviceAddress to sound,
video & watchdog device struts.
* src/conf/domain_conf.c: Hook up parsing/formatting for
virDomainDeviceAddress in sound, video & watchdog devices
---
src/conf/domain_conf.c | 85 ++++++++++++++++++++++++++++++++++++++++++-----
src/conf/domain_conf.h | 3 ++
2 files changed, 79 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9c0b8cf..a1eeb06 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2369,6 +2369,19 @@ virDomainSoundDefParseXML(virConnectPtr conn,
char *model;
virDomainSoundDefPtr def;
+ xmlNodePtr addr = NULL;
+ xmlNodePtr cur;
+
+ cur = node->children;
+ while (cur != NULL) {
+ if (cur->type == XML_ELEMENT_NODE) {
+ if ((addr == NULL) &&
+ xmlStrEqual(cur->name, BAD_CAST "address")) {
+ addr = cur;
+ }
+ }
+ cur = cur->next;
+ }
if (VIR_ALLOC(def) < 0) {
virReportOOMError(conn);
@@ -2382,6 +2395,10 @@ virDomainSoundDefParseXML(virConnectPtr conn,
goto error;
}
+ if (addr &&
+ virDomainDeviceAddressParseXML(conn, addr, &def->addr, flags) < 0)
+ goto error;
+
cleanup:
VIR_FREE(model);
@@ -2397,11 +2414,24 @@ error:
static virDomainWatchdogDefPtr
virDomainWatchdogDefParseXML(virConnectPtr conn,
const xmlNodePtr node,
- int flags ATTRIBUTE_UNUSED) {
+ int flags) {
char *model = NULL;
char *action = NULL;
virDomainWatchdogDefPtr def;
+ xmlNodePtr addr = NULL;
+ xmlNodePtr cur;
+
+ cur = node->children;
+ while (cur != NULL) {
+ if (cur->type == XML_ELEMENT_NODE) {
+ if ((addr == NULL) &&
+ xmlStrEqual(cur->name, BAD_CAST "address")) {
+ addr = cur;
+ }
+ }
+ cur = cur->next;
+ }
if (VIR_ALLOC (def) < 0) {
virReportOOMError (conn);
@@ -2433,6 +2463,10 @@ virDomainWatchdogDefParseXML(virConnectPtr conn,
}
}
+ if (addr && (def->model == VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB) &&
+ virDomainDeviceAddressParseXML(conn, addr, &def->addr, flags) < 0)
+ goto error;
+
cleanup:
VIR_FREE (action);
VIR_FREE (model);
@@ -2548,6 +2582,7 @@ virDomainVideoDefParseXML(virConnectPtr conn,
int flags ATTRIBUTE_UNUSED) {
virDomainVideoDefPtr def;
xmlNodePtr cur;
+ xmlNodePtr addr = NULL;
char *type = NULL;
char *heads = NULL;
char *vram = NULL;
@@ -2566,6 +2601,9 @@ virDomainVideoDefParseXML(virConnectPtr conn,
vram = virXMLPropString(cur, "vram");
heads = virXMLPropString(cur, "heads");
def->accel = virDomainVideoAccelDefParseXML(conn, cur);
+ } else if ((addr == NULL) &&
+ xmlStrEqual(cur->name, BAD_CAST "address")) {
+ addr = cur;
}
}
cur = cur->next;
@@ -2605,6 +2643,10 @@ virDomainVideoDefParseXML(virConnectPtr conn,
def->heads = 1;
}
+ if (addr &&
+ virDomainDeviceAddressParseXML(conn, addr, &def->addr, flags) < 0)
+ goto error;
+
VIR_FREE(type);
VIR_FREE(vram);
VIR_FREE(heads);
@@ -4645,7 +4687,8 @@ cleanup:
static int
virDomainSoundDefFormat(virConnectPtr conn,
virBufferPtr buf,
- virDomainSoundDefPtr def)
+ virDomainSoundDefPtr def,
+ int flags)
{
const char *model = virDomainSoundModelTypeToString(def->model);
@@ -4655,9 +4698,18 @@ virDomainSoundDefFormat(virConnectPtr conn,
return -1;
}
- virBufferVSprintf(buf, " <sound model='%s'/>\n",
+ virBufferVSprintf(buf, " <sound model='%s'",
model);
+ if (def->addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ virBufferAddLit(buf, ">\n");
+ if (virDomainDeviceAddressFormat(buf, &def->addr, flags) < 0)
+ return -1;
+ virBufferAddLit(buf, " </sound>\n");
+ } else {
+ virBufferAddLit(buf, "/>\n");
+ }
+
return 0;
}
@@ -4665,7 +4717,8 @@ virDomainSoundDefFormat(virConnectPtr conn,
static int
virDomainWatchdogDefFormat(virConnectPtr conn,
virBufferPtr buf,
- virDomainWatchdogDefPtr def)
+ virDomainWatchdogDefPtr def,
+ int flags)
{
const char *model = virDomainWatchdogModelTypeToString (def->model);
const char *action = virDomainWatchdogActionTypeToString (def->action);
@@ -4682,9 +4735,18 @@ virDomainWatchdogDefFormat(virConnectPtr conn,
return -1;
}
- virBufferVSprintf(buf, " <watchdog model='%s'
action='%s'/>\n",
+ virBufferVSprintf(buf, " <watchdog model='%s'
action='%s'",
model, action);
+ if (def->addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ virBufferAddLit(buf, ">\n");
+ if (virDomainDeviceAddressFormat(buf, &def->addr, flags) < 0)
+ return -1;
+ virBufferAddLit(buf, " </watchdog>\n");
+ } else {
+ virBufferAddLit(buf, "/>\n");
+ }
+
return 0;
}
@@ -4704,7 +4766,8 @@ virDomainVideoAccelDefFormat(virBufferPtr buf,
static int
virDomainVideoDefFormat(virConnectPtr conn,
virBufferPtr buf,
- virDomainVideoDefPtr def)
+ virDomainVideoDefPtr def,
+ int flags)
{
const char *model = virDomainVideoTypeToString(def->type);
@@ -4729,6 +4792,10 @@ virDomainVideoDefFormat(virConnectPtr conn,
virBufferAddLit(buf, "/>\n");
}
+ if (def->addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+ virDomainDeviceAddressFormat(buf, &def->addr, flags) < 0)
+ return -1;
+
virBufferAddLit(buf, " </video>\n");
return 0;
@@ -5128,11 +5195,11 @@ char *virDomainDefFormat(virConnectPtr conn,
}
for (n = 0 ; n < def->nsounds ; n++)
- if (virDomainSoundDefFormat(conn, &buf, def->sounds[n]) < 0)
+ if (virDomainSoundDefFormat(conn, &buf, def->sounds[n], flags) < 0)
goto cleanup;
for (n = 0 ; n < def->nvideos ; n++)
- if (virDomainVideoDefFormat(conn, &buf, def->videos[n]) < 0)
+ if (virDomainVideoDefFormat(conn, &buf, def->videos[n], flags) < 0)
goto cleanup;
for (n = 0 ; n < def->nhostdevs ; n++)
@@ -5140,7 +5207,7 @@ char *virDomainDefFormat(virConnectPtr conn,
goto cleanup;
if (def->watchdog)
- virDomainWatchdogDefFormat (conn, &buf, def->watchdog);
+ virDomainWatchdogDefFormat (conn, &buf, def->watchdog, flags);
virBufferAddLit(&buf, " </devices>\n");
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index d1c1b67..f06c2dc 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -348,6 +348,7 @@ typedef struct _virDomainSoundDef virDomainSoundDef;
typedef virDomainSoundDef *virDomainSoundDefPtr;
struct _virDomainSoundDef {
int model;
+ virDomainDeviceAddress addr;
};
enum virDomainWatchdogModel {
@@ -372,6 +373,7 @@ typedef virDomainWatchdogDef *virDomainWatchdogDefPtr;
struct _virDomainWatchdogDef {
int model;
int action;
+ virDomainDeviceAddress addr;
};
@@ -401,6 +403,7 @@ struct _virDomainVideoDef {
unsigned int vram;
unsigned int heads;
virDomainVideoAccelDefPtr accel;
+ virDomainDeviceAddress addr;
};
/* 3 possible graphics console modes */
--
1.6.5.2
4:22 p.m.
New subject: [libvirt] [PATCH 05/12] Clear dynamic values from domain config at shutdown
Some attributes in the guest config are only valid at runtime.
Although the XML parser/formatter are careful to avoid outputting
these attributes at the wrong time, there is still a risk that
the driver code may accidentally use them internally. We can
solve this by explicitly clearing all values
* src/conf/domain_conf.h, src/conf/domain_conf.c,
src/libvirt_private.syms: Add virDomainDefClearDynamicValues
* src/lxc/lxc_driver.c, src/qemu/qemu_driver.c,
src/uml/uml_driver.c: Call virDomainDefClearDynamicValues() when
any guest shuts down.
---
src/conf/domain_conf.c | 36 ++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 2 ++
src/libvirt_private.syms | 2 ++
src/lxc/lxc_driver.c | 2 ++
src/qemu/qemu_driver.c | 2 ++
src/uml/uml_driver.c | 2 ++
6 files changed, 46 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a1eeb06..533a1b0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5286,6 +5286,42 @@ error:
return NULL;
}
+static void virDomainDeviceAddressClearDynamicValues(virDomainDeviceAddressPtr addr)
+{
+ if (addr->mode == VIR_DOMAIN_DEVICE_ADDRESS_MODE_DYNAMIC)
+ memset(addr, 0, sizeof(*addr));
+}
+
+void virDomainDefClearDynamicValues(virDomainDefPtr def)
+{
+ int i;
+
+ def->id = -1;
+
+ for (i = 0; i < def->ngraphics ; i++) {
+ if (def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
+ def->graphics[i]->data.vnc.autoport)
+ def->graphics[i]->data.vnc.port = -1;
+ else if (def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_RDP &&
+ def->graphics[i]->data.rdp.autoport)
+ def->graphics[i]->data.rdp.port = -1;
+ }
+
+ for (i = 0 ; i < def->ndisks ; i++) {
+ virDomainDeviceAddressClearDynamicValues(&def->disks[i]->addr);
+ }
+ for (i = 0 ; i < def->nhostdevs ; i++) {
+ virDomainDeviceAddressClearDynamicValues(&def->hostdevs[i]->addr);
+ }
+ for (i = 0 ; i < def->nnets ; i++) {
+ virDomainDeviceAddressClearDynamicValues(&def->nets[i]->addr);
+ }
+
+ if (def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
+ VIR_FREE(def->seclabel.label);
+ VIR_FREE(def->seclabel.imagelabel);
+ }
+}
#ifndef PROXY
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f06c2dc..c93aed6 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -775,6 +775,8 @@ char *virDomainObjFormat(virConnectPtr conn,
virDomainObjPtr obj,
int flags);
+void virDomainDefClearDynamicValues(virDomainDefPtr def);
+
int virDomainCpuSetParse(virConnectPtr conn,
const char **str,
char sep,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 49df15c..285715d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -161,6 +161,8 @@ virDomainDeviceAddressIsValid;
virDomainDevicePCIAddressIsValid;
virDomainDeviceUSBAddressIsValid;
virDomainDeviceAddressClear;
+virDomainDeviceAddressTypeToString;
+virDomainDefClearDynamicValues;
# domain_event.h
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index c8e2dca..6a2fefb 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -749,6 +749,8 @@ static int lxcVmCleanup(virConnectPtr conn,
vm->newDef = NULL;
}
+ virDomainDefClearDynamicValues(vm->def);
+
return rc;
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index da4fae7..85cbaf7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2617,6 +2617,8 @@ retry:
vm->def->id = -1;
vm->newDef = NULL;
}
+
+ virDomainDefClearDynamicValues(vm->def);
}
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 48ef103..44d937f 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -962,6 +962,8 @@ static void umlShutdownVMDaemon(virConnectPtr conn ATTRIBUTE_UNUSED,
vm->def->id = -1;
vm->newDef = NULL;
}
+
+ virDomainDefClearDynamicValues(vm->def);
}
--
1.6.5.2
4:22 p.m.
New subject: [libvirt] [PATCH 06/12] Add new domain device: "controller"
From: Wolfgang Mauerer <wolfgang.mauerer(a)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" index="<num>">
<address type="pci" domain="0xNUM" bus="0xNUM"
slot="0xNUM"/>
</controller>
where type denotes the disk interface (scsi, ide,...), index
is an integer that identifies the controller for association
with disks, and the <address> element specifies the controller
address on the PCI bus as described in previous commits
The address element can be omitted; in this case, an address
will be assigned automatically.
Most of the code in this patch is from Wolfgang Mauerer's
previous disk controller series
---
src/conf/domain_conf.c | 207 +++++++++++++++++++++++++++++++++++++++++++++-
src/conf/domain_conf.h | 30 +++++++
src/libvirt_private.syms | 2 +
3 files changed, 238 insertions(+), 1 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 533a1b0..95a9d52 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -86,7 +86,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
"sound",
"video",
"hostdev",
- "watchdog")
+ "watchdog",
+ "controller")
VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
"none",
@@ -124,6 +125,12 @@ VIR_ENUM_IMPL(virDomainDiskCache, VIR_DOMAIN_DISK_CACHE_LAST,
"writethrough",
"writeback")
+VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST,
+ "ide",
+ "fdc",
+ "scsi",
+ "sata")
+
VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST,
"mount",
"block",
@@ -371,6 +378,14 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
VIR_FREE(def);
}
+void virDomainControllerDefFree(virDomainControllerDefPtr def)
+{
+ if (!def)
+ return;
+
+ VIR_FREE(def);
+}
+
void virDomainFSDefFree(virDomainFSDefPtr def)
{
if (!def)
@@ -523,6 +538,9 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def)
case VIR_DOMAIN_DEVICE_WATCHDOG:
virDomainWatchdogDefFree(def->data.watchdog);
break;
+ case VIR_DOMAIN_DEVICE_CONTROLLER:
+ virDomainControllerDefFree(def->data.controller);
+ break;
}
VIR_FREE(def);
@@ -556,6 +574,10 @@ void virDomainDefFree(virDomainDefPtr def)
virDomainDiskDefFree(def->disks[i]);
VIR_FREE(def->disks);
+ for (i = 0 ; i < def->ncontrollers ; i++)
+ virDomainControllerDefFree(def->controllers[i]);
+ VIR_FREE(def->controllers);
+
for (i = 0 ; i < def->nfss ; i++)
virDomainFSDefFree(def->fss[i]);
VIR_FREE(def->fss);
@@ -1390,6 +1412,76 @@ 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) {
+ virDomainControllerDefPtr def;
+ xmlNodePtr cur;
+ char *type = NULL;
+ char *idx = NULL;
+ xmlNodePtr addr = NULL;
+
+ if (VIR_ALLOC(def) < 0) {
+ virReportOOMError(conn);
+ return NULL;
+ }
+
+ type = virXMLPropString(node, "type");
+ if (type) {
+ if ((def->type = virDomainDiskBusTypeFromString(type)) < 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("unknown disk controller type
'%s'"), type);
+ goto error;
+ }
+ }
+
+ idx = virXMLPropString(node, "index");
+ if (idx) {
+ if (virStrToLong_i(idx, NULL, 10, &def->idx) < 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("cannot parse disk controller index %s"),
idx);
+ goto error;
+ }
+ }
+
+ cur = node->children;
+ while (cur != NULL) {
+ if (cur->type == XML_ELEMENT_NODE &&
+ addr == NULL) {
+ addr = cur;
+ }
+
+ cur = cur->next;
+ }
+
+ if (addr) {
+ if (virDomainDeviceAddressParseXML(conn, addr, &def->addr,flags) < 0)
+ goto error;
+ }
+
+ if (def->addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+ def->addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Disk controllers must use the 'pci' address
type"));
+ goto error;
+ }
+
+cleanup:
+ VIR_FREE(type);
+ VIR_FREE(idx);
+
+ 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
*/
@@ -3092,6 +3184,10 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virConnectPtr conn,
dev->type = VIR_DOMAIN_DEVICE_HOSTDEV;
if (!(dev->data.hostdev = virDomainHostdevDefParseXML(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 {
virDomainReportError(conn, VIR_ERR_XML_ERROR,
"%s", _("unknown device type"));
@@ -3164,6 +3260,59 @@ void virDomainDiskInsertPreAlloced(virDomainDefPtr def,
}
+int virDomainControllerInsert(virDomainDefPtr def,
+ virDomainControllerDefPtr controller)
+{
+
+ if (VIR_REALLOC_N(def->controllers, def->ncontrollers+1) < 0)
+ return -1;
+
+ virDomainControllerInsertPreAlloced(def, controller);
+
+ return 0;
+}
+
+void virDomainControllerInsertPreAlloced(virDomainDefPtr def,
+ virDomainControllerDefPtr controller)
+{
+ int i;
+ /* Tenatively plan to insert controller at the end. */
+ int insertAt = -1;
+
+ /* Then work backwards looking for controllers of
+ * the same type. If we find a controller with a
+ * index greater than the new one, insert at
+ * that position
+ */
+ for (i = (def->ncontrollers - 1) ; i >= 0 ; i--) {
+ /* If bus matches and current controller is after
+ * new controller, then new controller should go here */
+ if ((def->controllers[i]->type == controller->type) &&
+ (def->controllers[i]->idx > controller->idx)) {
+ insertAt = i;
+ } else if (def->controllers[i]->type == controller->type &&
+ insertAt == -1) {
+ /* Last controller with match bus is before the
+ * new controller, then put new controller just after
+ */
+ insertAt = i + 1;
+ }
+ }
+
+ /* No controllers with this bus yet, so put at end of list */
+ if (insertAt == -1)
+ insertAt = def->ncontrollers;
+
+ if (insertAt < def->ncontrollers)
+ memmove(def->controllers + insertAt + 1,
+ def->controllers + insertAt,
+ (sizeof(def->controllers[0]) * (def->ncontrollers-insertAt)));
+
+ def->controllers[insertAt] = controller;
+ def->ncontrollers++;
+}
+
+
#ifndef PROXY
static char *virDomainDefDefaultEmulator(virConnectPtr conn,
virDomainDefPtr def,
@@ -3479,6 +3628,25 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn,
}
VIR_FREE(nodes);
+ /* analysis of the controller devices */
+ 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;
+ }
+ VIR_FREE(nodes);
+
/* analysis of the filesystems */
if ((n = virXPathNodeSet(conn, "./devices/filesystem", ctxt, &nodes))
< 0) {
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
@@ -4387,6 +4555,36 @@ virDomainDiskDefFormat(virConnectPtr conn,
}
static int
+virDomainControllerDefFormat(virConnectPtr conn,
+ virBufferPtr buf,
+ virDomainControllerDefPtr def,
+ int flags)
+{
+ const char *type = virDomainControllerTypeToString(def->type);
+
+ if (!type) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("unexpected controller type %d"),
def->type);
+ return -1;
+ }
+
+ virBufferVSprintf(buf,
+ " <controller type='%s'
index='%d'",
+ type, def->idx);
+
+ if (def->addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ virBufferAddLit(buf, ">\n");
+ if (virDomainDeviceAddressFormat(buf, &def->addr, flags) < 0)
+ return -1;
+ virBufferAddLit(buf, " </controller>\n");
+ } else {
+ virBufferAddLit(buf, "/>\n");
+ }
+
+ return 0;
+}
+
+static int
virDomainFSDefFormat(virConnectPtr conn,
virBufferPtr buf,
virDomainFSDefPtr def)
@@ -5138,6 +5336,10 @@ char *virDomainDefFormat(virConnectPtr conn,
if (virDomainDiskDefFormat(conn, &buf, def->disks[n], flags) < 0)
goto cleanup;
+ for (n = 0 ; n < def->ncontrollers ; n++)
+ if (virDomainControllerDefFormat(conn, &buf, def->controllers[n], flags)
< 0)
+ goto cleanup;
+
for (n = 0 ; n < def->nfss ; n++)
if (virDomainFSDefFormat(conn, &buf, def->fss[n]) < 0)
goto cleanup;
@@ -5316,6 +5518,9 @@ void virDomainDefClearDynamicValues(virDomainDefPtr def)
for (i = 0 ; i < def->nnets ; i++) {
virDomainDeviceAddressClearDynamicValues(&def->nets[i]->addr);
}
+ for (i = 0 ; i < def->ncontrollers ; i++) {
+ virDomainDeviceAddressClearDynamicValues(&def->controllers[i]->addr);
+ }
if (def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
VIR_FREE(def->seclabel.label);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c93aed6..413647b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -176,6 +176,25 @@ struct _virDomainDiskDef {
};
+enum virDomainControllerType {
+ VIR_DOMAIN_CONTROLLER_TYPE_IDE,
+ VIR_DOMAIN_CONTROLLER_TYPE_FDC,
+ VIR_DOMAIN_CONTROLLER_TYPE_SCSI,
+ VIR_DOMAIN_CONTROLLER_TYPE_SATA,
+
+ VIR_DOMAIN_CONTROLLER_TYPE_LAST
+};
+
+/* Stores the virtual disk controller configuration */
+typedef struct _virDomainControllerDef virDomainControllerDef;
+typedef virDomainControllerDef *virDomainControllerDefPtr;
+struct _virDomainControllerDef {
+ int type;
+ int idx;
+ virDomainDeviceAddress addr;
+};
+
+
/* Two types of disk backends */
enum virDomainFSType {
VIR_DOMAIN_FS_TYPE_MOUNT, /* Better named 'bind' */
@@ -500,6 +519,7 @@ enum virDomainDeviceType {
VIR_DOMAIN_DEVICE_VIDEO,
VIR_DOMAIN_DEVICE_HOSTDEV,
VIR_DOMAIN_DEVICE_WATCHDOG,
+ VIR_DOMAIN_DEVICE_CONTROLLER,
VIR_DOMAIN_DEVICE_LAST,
};
@@ -510,6 +530,7 @@ struct _virDomainDeviceDef {
int type;
union {
virDomainDiskDefPtr disk;
+ virDomainControllerDefPtr controller;
virDomainFSDefPtr fs;
virDomainNetDefPtr net;
virDomainInputDefPtr input;
@@ -622,6 +643,9 @@ struct _virDomainDef {
int ndisks;
virDomainDiskDefPtr *disks;
+ int ncontrollers;
+ virDomainControllerDefPtr *controllers;
+
int nfss;
virDomainFSDefPtr *fss;
@@ -704,6 +728,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);
@@ -790,6 +815,10 @@ int virDomainDiskInsert(virDomainDefPtr def,
virDomainDiskDefPtr disk);
void virDomainDiskInsertPreAlloced(virDomainDefPtr def,
virDomainDiskDefPtr disk);
+int virDomainControllerInsert(virDomainDefPtr def,
+ virDomainControllerDefPtr controller);
+void virDomainControllerInsertPreAlloced(virDomainDefPtr def,
+ virDomainControllerDefPtr controller);
int virDomainSaveXML(virConnectPtr conn,
const char *configDir,
@@ -871,6 +900,7 @@ VIR_ENUM_DECL(virDomainDisk)
VIR_ENUM_DECL(virDomainDiskDevice)
VIR_ENUM_DECL(virDomainDiskBus)
VIR_ENUM_DECL(virDomainDiskCache)
+VIR_ENUM_DECL(virDomainController)
VIR_ENUM_DECL(virDomainFS)
VIR_ENUM_DECL(virDomainNet)
VIR_ENUM_DECL(virDomainChrTarget)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 285715d..8622e5b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -109,6 +109,8 @@ virDomainDiskDefFree;
virDomainDiskDeviceTypeToString;
virDomainDiskInsert;
virDomainDiskInsertPreAlloced;
+virDomainControllerInsert;
+virDomainControllerInsertPreAlloced;
virDomainFindByID;
virDomainFindByName;
virDomainFindByUUID;
--
1.6.5.2
4:22 p.m.
New subject: [libvirt] [PATCH 07/12] Convert monitor over to use virDomainDeviceAddress
Convert the QEMU monitor APIs over to use virDomainDeviceAddress
structs for passing addresses in/out, instead of individual bits
* src/qemu/qemu_driver.c, src/qemu/qemu_monitor.c,
src/qemu/qemu_monitor.h, src/qemu/qemu_monitor_text.c,
src/qemu/qemu_monitor_text.h: Change monitor hotplug APIs to
take an explicit address ptr for all host/guest addresses
---
src/qemu/qemu_driver.c | 38 ++++++++--------------
src/qemu/qemu_monitor.c | 69 +++++++++++++-----------------------------
src/qemu/qemu_monitor.h | 24 ++++-----------
src/qemu/qemu_monitor_json.c | 60 ++++++++++++++++--------------------
src/qemu/qemu_monitor_json.h | 25 ++++-----------
src/qemu/qemu_monitor_text.c | 61 +++++++++++++-----------------------
src/qemu/qemu_monitor_text.h | 25 ++++-----------
7 files changed, 102 insertions(+), 200 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 85cbaf7..321cd4e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4930,6 +4930,7 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
int i, ret;
const char* type = virDomainDiskBusTypeToString(dev->data.disk->bus);
qemuDomainObjPrivatePtr priv = vm->privateData;
+ virDomainDevicePCIAddress guestAddr;
for (i = 0 ; i < vm->def->ndisks ; i++) {
if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) {
@@ -4948,13 +4949,12 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
ret = qemuMonitorAddPCIDisk(priv->mon,
dev->data.disk->src,
type,
- &dev->data.disk->addr.data.pci.domain,
- &dev->data.disk->addr.data.pci.bus,
- &dev->data.disk->addr.data.pci.slot);
+ &guestAddr);
qemuDomainObjExitMonitorWithDriver(driver, vm);
if (ret == 0) {
dev->data.disk->addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+ memcpy(&dev->data.disk->addr.data.pci, &guestAddr,
sizeof(guestAddr));
virDomainDiskInsertPreAlloced(vm->def, dev->data.disk);
}
@@ -5011,6 +5011,7 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn,
char *nicstr = NULL;
char *netstr = NULL;
int ret = -1;
+ virDomainDevicePCIAddress guestAddr;
if (!(qemuCmdFlags & QEMUD_CMD_FLAG_HOST_NET_ADD)) {
qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT, "%s",
@@ -5079,13 +5080,12 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn,
qemuDomainObjEnterMonitorWithDriver(driver, vm);
if (qemuMonitorAddPCINetwork(priv->mon, nicstr,
- &net->addr.data.pci.domain,
- &net->addr.data.pci.bus,
- &net->addr.data.pci.slot) < 0) {
+ &guestAddr) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
goto try_remove;
}
net->addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+ memcpy(&net->addr.data.pci, &guestAddr, sizeof(guestAddr));
qemuDomainObjExitMonitorWithDriver(driver, vm);
ret = 0;
@@ -5137,6 +5137,7 @@ static int qemudDomainAttachHostPciDevice(virConnectPtr conn,
virDomainHostdevDefPtr hostdev = dev->data.hostdev;
pciDevice *pci;
int ret;
+ virDomainDevicePCIAddress guestAddr;
if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0) {
virReportOOMError(conn);
@@ -5164,17 +5165,13 @@ static int qemudDomainAttachHostPciDevice(virConnectPtr conn,
qemuDomainObjEnterMonitorWithDriver(driver, vm);
ret = qemuMonitorAddPCIHostDevice(priv->mon,
- hostdev->source.subsys.u.pci.domain,
- hostdev->source.subsys.u.pci.bus,
- hostdev->source.subsys.u.pci.slot,
- hostdev->source.subsys.u.pci.function,
- &hostdev->addr.data.pci.domain,
- &hostdev->addr.data.pci.bus,
- &hostdev->addr.data.pci.slot);
+ &hostdev->source.subsys.u.pci,
+ &guestAddr);
qemuDomainObjExitMonitorWithDriver(driver, vm);
if (ret < 0)
goto error;
hostdev->addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+ memcpy(&hostdev->addr.data.pci, &guestAddr, sizeof(guestAddr));
vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
@@ -5206,8 +5203,7 @@ static int qemudDomainAttachHostUsbDevice(virConnectPtr conn,
dev->data.hostdev->source.subsys.u.usb.product);
} else {
ret = qemuMonitorAddUSBDeviceExact(priv->mon,
-
dev->data.hostdev->source.subsys.u.usb.addr.bus,
-
dev->data.hostdev->source.subsys.u.usb.addr.dev);
+
&dev->data.hostdev->source.subsys.u.usb.addr);
}
qemuDomainObjExitMonitorWithDriver(driver, vm);
@@ -5413,9 +5409,7 @@ static int qemudDomainDetachPciDiskDevice(virConnectPtr conn,
qemuDomainObjEnterMonitorWithDriver(driver, vm);
if (qemuMonitorRemovePCIDevice(priv->mon,
- detach->addr.data.pci.domain,
- detach->addr.data.pci.bus,
- detach->addr.data.pci.slot) < 0) {
+ &detach->addr.data.pci) < 0) {
qemuDomainObjExitMonitor(vm);
goto cleanup;
}
@@ -5485,9 +5479,7 @@ qemudDomainDetachNetDevice(virConnectPtr conn,
qemuDomainObjEnterMonitorWithDriver(driver, vm);
if (qemuMonitorRemovePCIDevice(priv->mon,
- detach->addr.data.pci.domain,
- detach->addr.data.pci.bus,
- detach->addr.data.pci.slot) < 0) {
+ &detach->addr.data.pci) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
goto cleanup;
}
@@ -5574,9 +5566,7 @@ static int qemudDomainDetachHostPciDevice(virConnectPtr conn,
qemuDomainObjEnterMonitorWithDriver(driver, vm);
if (qemuMonitorRemovePCIDevice(priv->mon,
- detach->addr.data.pci.domain,
- detach->addr.data.pci.bus,
- detach->addr.data.pci.slot) < 0) {
+ &detach->addr.data.pci) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
return -1;
}
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 103cf28..ded1622 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1087,16 +1087,15 @@ int qemuMonitorAddUSBDisk(qemuMonitorPtr mon,
int qemuMonitorAddUSBDeviceExact(qemuMonitorPtr mon,
- int bus,
- int dev)
+ virDomainDeviceUSBAddress *hostAddr)
{
int ret;
- DEBUG("mon=%p, fd=%d bus=%d dev=%d", mon, mon->fd, bus, dev);
+ DEBUG("mon=%p, fd=%d bus=%d dev=%d", mon, mon->fd, hostAddr->bus,
hostAddr->dev);
if (mon->json)
- ret = qemuMonitorJSONAddUSBDeviceExact(mon, bus, dev);
+ ret = qemuMonitorJSONAddUSBDeviceExact(mon, hostAddr);
else
- ret = qemuMonitorTextAddUSBDeviceExact(mon, bus, dev);
+ ret = qemuMonitorTextAddUSBDeviceExact(mon, hostAddr);
return ret;
}
@@ -1117,33 +1116,18 @@ int qemuMonitorAddUSBDeviceMatch(qemuMonitorPtr mon,
int qemuMonitorAddPCIHostDevice(qemuMonitorPtr mon,
- unsigned hostDomain,
- unsigned hostBus,
- unsigned hostSlot,
- unsigned hostFunction,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot)
+ virDomainDevicePCIAddress *hostAddr,
+ virDomainDevicePCIAddress *guestAddr)
{
int ret;
DEBUG("mon=%p, fd=%d domain=%d bus=%d slot=%d function=%d",
mon, mon->fd,
- hostDomain, hostBus, hostSlot, hostFunction);
+ hostAddr->domain, hostAddr->bus, hostAddr->slot,
hostAddr->function);
if (mon->json)
- ret = qemuMonitorJSONAddPCIHostDevice(mon, hostDomain,
- hostBus, hostSlot,
- hostFunction,
- guestDomain,
- guestBus,
- guestSlot);
+ ret = qemuMonitorJSONAddPCIHostDevice(mon, hostAddr, guestAddr);
else
- ret = qemuMonitorTextAddPCIHostDevice(mon, hostDomain,
- hostBus, hostSlot,
- hostFunction,
- guestDomain,
- guestBus,
- guestSlot);
+ ret = qemuMonitorTextAddPCIHostDevice(mon, hostAddr, guestAddr);
return ret;
}
@@ -1151,58 +1135,47 @@ int qemuMonitorAddPCIHostDevice(qemuMonitorPtr mon,
int qemuMonitorAddPCIDisk(qemuMonitorPtr mon,
const char *path,
const char *bus,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot)
+ virDomainDevicePCIAddress *guestAddr)
{
int ret;
DEBUG("mon=%p, fd=%d path=%s bus=%s",
mon, mon->fd, path, bus);
if (mon->json)
- ret = qemuMonitorJSONAddPCIDisk(mon, path, bus,
- guestDomain, guestBus, guestSlot);
+ ret = qemuMonitorJSONAddPCIDisk(mon, path, bus, guestAddr);
else
- ret = qemuMonitorTextAddPCIDisk(mon, path, bus,
- guestDomain, guestBus, guestSlot);
+ ret = qemuMonitorTextAddPCIDisk(mon, path, bus, guestAddr);
return ret;
}
int qemuMonitorAddPCINetwork(qemuMonitorPtr mon,
const char *nicstr,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot)
+ virDomainDevicePCIAddress *guestAddr)
{
int ret;
DEBUG("mon=%p, fd=%d nicstr=%s", mon, mon->fd, nicstr);
if (mon->json)
- ret = qemuMonitorJSONAddPCINetwork(mon, nicstr, guestDomain,
- guestBus, guestSlot);
+ ret = qemuMonitorJSONAddPCINetwork(mon, nicstr, guestAddr);
else
- ret = qemuMonitorTextAddPCINetwork(mon, nicstr, guestDomain,
- guestBus, guestSlot);
+ ret = qemuMonitorTextAddPCINetwork(mon, nicstr, guestAddr);
return ret;
}
int qemuMonitorRemovePCIDevice(qemuMonitorPtr mon,
- unsigned guestDomain,
- unsigned guestBus,
- unsigned guestSlot)
+ virDomainDevicePCIAddress *guestAddr)
{
int ret;
- DEBUG("mon=%p, fd=%d domain=%d bus=%d slot=%d",
- mon, mon->fd, guestDomain, guestBus, guestSlot);
+ DEBUG("mon=%p, fd=%d domain=%d bus=%d slot=%d function=%d",
+ mon, mon->fd, guestAddr->domain, guestAddr->bus,
+ guestAddr->slot, guestAddr->function);
if (mon->json)
- ret = qemuMonitorJSONRemovePCIDevice(mon, guestDomain,
- guestBus, guestSlot);
+ ret = qemuMonitorJSONRemovePCIDevice(mon, guestAddr);
else
- ret = qemuMonitorTextRemovePCIDevice(mon, guestDomain,
- guestBus, guestSlot);
+ ret = qemuMonitorTextRemovePCIDevice(mon, guestAddr);
return ret;
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 8b1e3a3..46f412e 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -213,21 +213,15 @@ int qemuMonitorAddUSBDisk(qemuMonitorPtr mon,
const char *path);
int qemuMonitorAddUSBDeviceExact(qemuMonitorPtr mon,
- int bus,
- int dev);
+ virDomainDeviceUSBAddress *hostAddr);
int qemuMonitorAddUSBDeviceMatch(qemuMonitorPtr mon,
int vendor,
int product);
int qemuMonitorAddPCIHostDevice(qemuMonitorPtr mon,
- unsigned hostDomain,
- unsigned hostBus,
- unsigned hostSlot,
- unsigned hostFunction,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot);
+ virDomainDevicePCIAddress *hostAddr,
+ virDomainDevicePCIAddress *guestAddr);
/* XXX disk driver type eg, qcow/etc.
* XXX cache mode
@@ -235,23 +229,17 @@ int qemuMonitorAddPCIHostDevice(qemuMonitorPtr mon,
int qemuMonitorAddPCIDisk(qemuMonitorPtr mon,
const char *path,
const char *bus,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot);
+ virDomainDevicePCIAddress *guestAddr);
/* XXX do we really want to hardcode 'nicstr' as the
* sendable item here
*/
int qemuMonitorAddPCINetwork(qemuMonitorPtr mon,
const char *nicstr,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot);
+ virDomainDevicePCIAddress *guestAddr);
int qemuMonitorRemovePCIDevice(qemuMonitorPtr mon,
- unsigned guestDomain,
- unsigned guestBus,
- unsigned guestSlot);
+ virDomainDevicePCIAddress *guestAddr);
int qemuMonitorSendFileHandle(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 03562e8..48eca13 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1166,13 +1166,12 @@ int qemuMonitorJSONAddUSBDisk(qemuMonitorPtr mon,
int qemuMonitorJSONAddUSBDeviceExact(qemuMonitorPtr mon,
- int bus,
- int dev)
+ virDomainDeviceUSBAddress *hostAddr)
{
int ret;
char *addr;
- if (virAsprintf(&addr, "host:%.3d.%.3d", bus, dev) < 0) {
+ if (virAsprintf(&addr, "host:%.3d.%.3d", hostAddr->bus,
hostAddr->dev) < 0) {
virReportOOMError(NULL);
return -1;
}
@@ -1206,9 +1205,7 @@ int qemuMonitorJSONAddUSBDeviceMatch(qemuMonitorPtr mon,
/* XXX qemu also returns a 'function' number now */
static int
qemuMonitorJSONGetGuestAddress(virJSONValuePtr reply,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot)
+ virDomainDevicePCIAddress *guestAddr)
{
virJSONValuePtr addr;
@@ -1219,47 +1216,48 @@ qemuMonitorJSONGetGuestAddress(virJSONValuePtr reply,
return -1;
}
- if (virJSONValueObjectGetNumberUint(addr, "domain", guestDomain) < 0) {
+ if (virJSONValueObjectGetNumberUint(addr, "domain",
&guestAddr->domain) < 0) {
qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("pci_add reply was missing device domain number"));
return -1;
}
- if (virJSONValueObjectGetNumberUint(addr, "bus", guestBus) < 0) {
+ if (virJSONValueObjectGetNumberUint(addr, "bus", &guestAddr->bus)
< 0) {
qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("pci_add reply was missing device bus number"));
return -1;
}
- if (virJSONValueObjectGetNumberUint(addr, "slot", guestSlot) < 0) {
+ if (virJSONValueObjectGetNumberUint(addr, "slot", &guestAddr->slot)
< 0) {
qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("pci_add reply was missing device slot number"));
return -1;
}
+ if (virJSONValueObjectGetNumberUint(addr, "function",
&guestAddr->function) < 0) {
+ qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("pci_add reply was missing device function
number"));
+ return -1;
+ }
+
return 0;
}
int qemuMonitorJSONAddPCIHostDevice(qemuMonitorPtr mon,
- unsigned hostDomain ATTRIBUTE_UNUSED,
- unsigned hostBus,
- unsigned hostSlot,
- unsigned hostFunction,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot)
+ virDomainDevicePCIAddress *hostAddr,
+ virDomainDevicePCIAddress *guestAddr)
{
int ret;
virJSONValuePtr cmd;
virJSONValuePtr reply = NULL;
char *dev;
- *guestDomain = *guestBus = *guestSlot = 0;
+ memset(guestAddr, 0, sizeof(*guestAddr));
/* XXX hostDomain */
if (virAsprintf(&dev, "host=%.2x:%.2x.%.1x",
- hostBus, hostSlot, hostFunction) < 0) {
+ hostAddr->bus, hostAddr->slot, hostAddr->function) < 0)
{
virReportOOMError(NULL);
return -1;
}
@@ -1279,7 +1277,7 @@ int qemuMonitorJSONAddPCIHostDevice(qemuMonitorPtr mon,
ret = qemuMonitorJSONCheckError(cmd, reply);
if (ret == 0 &&
- qemuMonitorJSONGetGuestAddress(reply, guestDomain, guestBus, guestSlot) < 0)
+ qemuMonitorJSONGetGuestAddress(reply, guestAddr) < 0)
ret = -1;
virJSONValueFree(cmd);
@@ -1291,15 +1289,14 @@ int qemuMonitorJSONAddPCIHostDevice(qemuMonitorPtr mon,
int qemuMonitorJSONAddPCIDisk(qemuMonitorPtr mon,
const char *path,
const char *bus,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot) {
+ virDomainDevicePCIAddress *guestAddr)
+{
int ret;
virJSONValuePtr cmd;
virJSONValuePtr reply = NULL;
char *dev;
- *guestDomain = *guestBus = *guestSlot = 0;
+ memset(guestAddr, 0, sizeof(*guestAddr));
if (virAsprintf(&dev, "file=%s,if=%s", path, bus) < 0) {
virReportOOMError(NULL);
@@ -1321,7 +1318,7 @@ int qemuMonitorJSONAddPCIDisk(qemuMonitorPtr mon,
ret = qemuMonitorJSONCheckError(cmd, reply);
if (ret == 0 &&
- qemuMonitorJSONGetGuestAddress(reply, guestDomain, guestBus, guestSlot) < 0)
+ qemuMonitorJSONGetGuestAddress(reply, guestAddr) < 0)
ret = -1;
virJSONValueFree(cmd);
@@ -1332,9 +1329,7 @@ int qemuMonitorJSONAddPCIDisk(qemuMonitorPtr mon,
int qemuMonitorJSONAddPCINetwork(qemuMonitorPtr mon,
const char *nicstr,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot)
+ virDomainDevicePCIAddress *guestAddr)
{
int ret;
virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("pci_add",
@@ -1344,7 +1339,7 @@ int qemuMonitorJSONAddPCINetwork(qemuMonitorPtr mon,
NULL);
virJSONValuePtr reply = NULL;
- *guestDomain = *guestBus = *guestSlot = 0;
+ memset(guestAddr, 0, sizeof(*guestAddr));
if (!cmd)
return -1;
@@ -1355,7 +1350,7 @@ int qemuMonitorJSONAddPCINetwork(qemuMonitorPtr mon,
ret = qemuMonitorJSONCheckError(cmd, reply);
if (ret == 0 &&
- qemuMonitorJSONGetGuestAddress(reply, guestDomain, guestBus, guestSlot) < 0)
+ qemuMonitorJSONGetGuestAddress(reply, guestAddr) < 0)
ret = -1;
virJSONValueFree(cmd);
@@ -1365,17 +1360,16 @@ int qemuMonitorJSONAddPCINetwork(qemuMonitorPtr mon,
int qemuMonitorJSONRemovePCIDevice(qemuMonitorPtr mon,
- unsigned guestDomain,
- unsigned guestBus,
- unsigned guestSlot)
+ virDomainDevicePCIAddress *guestAddr)
{
int ret;
virJSONValuePtr cmd;
virJSONValuePtr reply = NULL;
char *addr;
+ /* XXX what about function ? */
if (virAsprintf(&addr, "%.4x:%.2x:%.2x",
- guestDomain, guestBus, guestSlot) < 0) {
+ guestAddr->domain, guestAddr->bus, guestAddr->slot) < 0)
{
virReportOOMError(NULL);
return -1;
}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 62a88c0..9df96f5 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -104,40 +104,27 @@ int qemuMonitorJSONAddUSBDisk(qemuMonitorPtr mon,
const char *path);
int qemuMonitorJSONAddUSBDeviceExact(qemuMonitorPtr mon,
- int bus,
- int dev);
+ virDomainDeviceUSBAddress *hostAddr);
int qemuMonitorJSONAddUSBDeviceMatch(qemuMonitorPtr mon,
int vendor,
int product);
int qemuMonitorJSONAddPCIHostDevice(qemuMonitorPtr mon,
- unsigned hostDomain,
- unsigned hostBus,
- unsigned hostSlot,
- unsigned hostFunction,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot);
+ virDomainDevicePCIAddress *hostAddr,
+ virDomainDevicePCIAddress *guestAddr);
int qemuMonitorJSONAddPCIDisk(qemuMonitorPtr mon,
const char *path,
const char *bus,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot);
+ virDomainDevicePCIAddress *guestAddr);
int qemuMonitorJSONAddPCINetwork(qemuMonitorPtr mon,
const char *nicstr,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot);
+ virDomainDevicePCIAddress *guestAddr);
int qemuMonitorJSONRemovePCIDevice(qemuMonitorPtr mon,
- unsigned guestDomain,
- unsigned guestBus,
- unsigned guestSlot);
-
+ virDomainDevicePCIAddress *guestAddr);
int qemuMonitorJSONSendFileHandle(qemuMonitorPtr mon,
const char *fdname,
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 72cb2bb..27c213d 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -1159,13 +1159,12 @@ cleanup:
int qemuMonitorTextAddUSBDeviceExact(qemuMonitorPtr mon,
- int bus,
- int dev)
+ virDomainDeviceUSBAddress *hostAddr)
{
int ret;
char *addr;
- if (virAsprintf(&addr, "host:%.3d.%.3d", bus, dev) < 0) {
+ if (virAsprintf(&addr, "host:%.3d.%.3d", hostAddr->bus,
hostAddr->dev) < 0) {
virReportOOMError(NULL);
return -1;
}
@@ -1198,9 +1197,7 @@ int qemuMonitorTextAddUSBDeviceMatch(qemuMonitorPtr mon,
static int
qemuMonitorTextParsePciAddReply(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
const char *reply,
- unsigned *domain,
- unsigned *bus,
- unsigned *slot)
+ virDomainDevicePCIAddress *addr)
{
char *s, *e;
@@ -1217,7 +1214,7 @@ qemuMonitorTextParsePciAddReply(qemuMonitorPtr mon
ATTRIBUTE_UNUSED,
if (STRPREFIX(s, "domain ")) {
s += strlen("domain ");
- if (virStrToLong_ui(s, &e, 10, domain) == -1) {
+ if (virStrToLong_ui(s, &e, 10, &addr->domain) == -1) {
VIR_WARN(_("Unable to parse domain number '%s'\n"), s);
return -1;
}
@@ -1235,7 +1232,7 @@ qemuMonitorTextParsePciAddReply(qemuMonitorPtr mon
ATTRIBUTE_UNUSED,
}
s += strlen("bus ");
- if (virStrToLong_ui(s, &e, 10, bus) == -1) {
+ if (virStrToLong_ui(s, &e, 10, &addr->bus) == -1) {
VIR_WARN(_("Unable to parse bus number '%s'\n"), s);
return -1;
}
@@ -1252,7 +1249,7 @@ qemuMonitorTextParsePciAddReply(qemuMonitorPtr mon
ATTRIBUTE_UNUSED,
}
s += strlen("slot ");
- if (virStrToLong_ui(s, &e, 10, slot) == -1) {
+ if (virStrToLong_ui(s, &e, 10, &addr->slot) == -1) {
VIR_WARN(_("Unable to parse slot number '%s'\n"), s);
return -1;
}
@@ -1262,23 +1259,18 @@ qemuMonitorTextParsePciAddReply(qemuMonitorPtr mon
ATTRIBUTE_UNUSED,
int qemuMonitorTextAddPCIHostDevice(qemuMonitorPtr mon,
- unsigned hostDomain ATTRIBUTE_UNUSED,
- unsigned hostBus,
- unsigned hostSlot,
- unsigned hostFunction,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot)
+ virDomainDevicePCIAddress *hostAddr,
+ virDomainDevicePCIAddress *guestAddr)
{
char *cmd;
char *reply = NULL;
int ret = -1;
- *guestDomain = *guestBus = *guestSlot = 0;
+ memset(guestAddr, 0, sizeof(*guestAddr));
- /* XXX hostDomain */
+ /* XXX hostAddr->domain */
if (virAsprintf(&cmd, "pci_add pci_addr=auto host
host=%.2x:%.2x.%.1x",
- hostBus, hostSlot, hostFunction) < 0) {
+ hostAddr->bus, hostAddr->slot, hostAddr->function) < 0)
{
virReportOOMError(NULL);
goto cleanup;
}
@@ -1295,10 +1287,7 @@ int qemuMonitorTextAddPCIHostDevice(qemuMonitorPtr mon,
goto cleanup;
}
- if (qemuMonitorTextParsePciAddReply(mon, reply,
- guestDomain,
- guestBus,
- guestSlot) < 0) {
+ if (qemuMonitorTextParsePciAddReply(mon, reply, guestAddr) < 0) {
qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED,
_("parsing pci_add reply failed: %s"), reply);
goto cleanup;
@@ -1316,9 +1305,8 @@ cleanup:
int qemuMonitorTextAddPCIDisk(qemuMonitorPtr mon,
const char *path,
const char *bus,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot) {
+ virDomainDevicePCIAddress *guestAddr)
+{
char *cmd = NULL;
char *reply = NULL;
char *safe_path = NULL;
@@ -1344,8 +1332,7 @@ try_command:
goto cleanup;
}
- if (qemuMonitorTextParsePciAddReply(mon, reply,
- guestDomain, guestBus, guestSlot) < 0) {
+ if (qemuMonitorTextParsePciAddReply(mon, reply, guestAddr) < 0) {
if (!tryOldSyntax && strstr(reply, "invalid char in
expression")) {
VIR_FREE(reply);
VIR_FREE(cmd);
@@ -1370,9 +1357,7 @@ cleanup:
int qemuMonitorTextAddPCINetwork(qemuMonitorPtr mon,
const char *nicstr,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot)
+ virDomainDevicePCIAddress *guestAddr)
{
char *cmd;
char *reply = NULL;
@@ -1389,8 +1374,7 @@ int qemuMonitorTextAddPCINetwork(qemuMonitorPtr mon,
goto cleanup;
}
- if (qemuMonitorTextParsePciAddReply(mon, reply,
- guestDomain, guestBus, guestSlot) < 0) {
+ if (qemuMonitorTextParsePciAddReply(mon, reply, guestAddr) < 0) {
qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED,
_("parsing pci_add reply failed: %s"), reply);
goto cleanup;
@@ -1406,9 +1390,7 @@ cleanup:
int qemuMonitorTextRemovePCIDevice(qemuMonitorPtr mon,
- unsigned guestDomain,
- unsigned guestBus,
- unsigned guestSlot)
+ virDomainDevicePCIAddress *guestAddr)
{
char *cmd = NULL;
char *reply = NULL;
@@ -1417,13 +1399,14 @@ int qemuMonitorTextRemovePCIDevice(qemuMonitorPtr mon,
try_command:
if (tryOldSyntax) {
- if (virAsprintf(&cmd, "pci_del 0 %.2x", guestSlot) < 0) {
+ if (virAsprintf(&cmd, "pci_del 0 %.2x", guestAddr->slot) < 0)
{
virReportOOMError(NULL);
goto cleanup;
}
} else {
+ /* XXX function ? */
if (virAsprintf(&cmd, "pci_del pci_addr=%.4x:%.2x:%.2x",
- guestDomain, guestBus, guestSlot) < 0) {
+ guestAddr->domain, guestAddr->bus, guestAddr->slot) <
0) {
virReportOOMError(NULL);
goto cleanup;
}
@@ -1451,7 +1434,7 @@ try_command:
strstr(reply, "Invalid pci address")) {
qemudReportError (NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED,
_("failed to detach PCI device, invalid address
%.4x:%.2x:%.2x: %s"),
- guestDomain, guestBus, guestSlot, reply);
+ guestAddr->domain, guestAddr->bus, guestAddr->slot,
reply);
goto cleanup;
}
diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h
index bc52ad2..f304795 100644
--- a/src/qemu/qemu_monitor_text.h
+++ b/src/qemu/qemu_monitor_text.h
@@ -104,40 +104,27 @@ int qemuMonitorTextAddUSBDisk(qemuMonitorPtr mon,
const char *path);
int qemuMonitorTextAddUSBDeviceExact(qemuMonitorPtr mon,
- int bus,
- int dev);
+ virDomainDeviceUSBAddress *hostAddr);
int qemuMonitorTextAddUSBDeviceMatch(qemuMonitorPtr mon,
int vendor,
int product);
int qemuMonitorTextAddPCIHostDevice(qemuMonitorPtr mon,
- unsigned hostDomain,
- unsigned hostBus,
- unsigned hostSlot,
- unsigned hostFunction,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot);
+ virDomainDevicePCIAddress *hostAddr,
+ virDomainDevicePCIAddress *guestAddr);
int qemuMonitorTextAddPCIDisk(qemuMonitorPtr mon,
const char *path,
const char *bus,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot);
+ virDomainDevicePCIAddress *guestAddr);
int qemuMonitorTextAddPCINetwork(qemuMonitorPtr mon,
const char *nicstr,
- unsigned *guestDomain,
- unsigned *guestBus,
- unsigned *guestSlot);
+ virDomainDevicePCIAddress *guestAddr);
int qemuMonitorTextRemovePCIDevice(qemuMonitorPtr mon,
- unsigned guestDomain,
- unsigned guestBus,
- unsigned guestSlot);
-
+ virDomainDevicePCIAddress *guestAddr);
int qemuMonitorTextSendFileHandle(qemuMonitorPtr mon,
const char *fdname,
--
1.6.5.2
4:22 p.m.
New subject: [libvirt] [PATCH 08/12] Split code for building QEMU -drive arg in separate method
To enable it to be called from multiple locations, split out
the code for building the -drive arg string
* src/qemu/qemu_conf.c, src/qemu/qemu_conf.h: Add qemuBuildDriveStr
for building -drive arg string
---
src/qemu/qemu_conf.c | 188 ++++++++++++++++++++++++++++----------------------
src/qemu/qemu_conf.h | 7 ++
2 files changed, 112 insertions(+), 83 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 39d9314..27faa3f 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1313,6 +1313,110 @@ qemuAssignNetNames(virDomainDefPtr def,
return 0;
}
+#define QEMU_SERIAL_PARAM_ACCEPTED_CHARS \
+ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_"
+
+static int
+qemuSafeSerialParamValue(virConnectPtr conn,
+ const char *value)
+{
+ if (strspn(value, QEMU_SERIAL_PARAM_ACCEPTED_CHARS) != strlen (value)) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("driver serial '%s' contains unsafe
characters"),
+ value);
+ return -1;
+ }
+
+ return 0;
+}
+
+
+int
+qemuBuildDriveStr(virConnectPtr conn,
+ virDomainDiskDefPtr disk,
+ int bootable,
+ int qemuCmdFlags,
+ char **str)
+{
+ virBuffer opt = VIR_BUFFER_INITIALIZER;
+ const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
+ int idx = virDiskNameToIndex(disk->dst);
+ char *optstr = NULL;
+
+ if (idx < 0) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("unsupported disk type '%s'"),
disk->dst);
+ goto error;
+ }
+
+ if (disk->src) {
+ if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) {
+ /* QEMU only supports magic FAT format for now */
+ if (disk->driverType &&
+ STRNEQ(disk->driverType, "fat")) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("unsupported disk driver type for
'%s'"),
+ disk->driverType);
+ goto error;
+ }
+ if (!disk->readonly) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
"%s",
+ _("cannot create virtual FAT disks in read-write
mode"));
+ goto error;
+ }
+ if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
+ virBufferVSprintf(&opt, "file=fat:floppy:%s,",
disk->src);
+ else
+ virBufferVSprintf(&opt, "file=fat:%s,", disk->src);
+ } else {
+ virBufferVSprintf(&opt, "file=%s,", disk->src);
+ }
+ }
+ virBufferVSprintf(&opt, "if=%s", bus);
+ if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
+ virBufferAddLit(&opt, ",media=cdrom");
+ virBufferVSprintf(&opt, ",index=%d", idx);
+ if (bootable &&
+ disk->device == VIR_DOMAIN_DISK_DEVICE_DISK)
+ virBufferAddLit(&opt, ",boot=on");
+ if (disk->driverType &&
+ disk->type != VIR_DOMAIN_DISK_TYPE_DIR &&
+ qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_FORMAT)
+ virBufferVSprintf(&opt, ",format=%s", disk->driverType);
+ if (disk->serial &&
+ (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_SERIAL)) {
+ if (qemuSafeSerialParamValue(conn, disk->serial) < 0)
+ goto error;
+ virBufferVSprintf(&opt, ",serial=%s", disk->serial);
+ }
+
+ if (disk->cachemode) {
+ const char *mode =
+ (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_CACHE_V2) ?
+ qemuDiskCacheV2TypeToString(disk->cachemode) :
+ qemuDiskCacheV1TypeToString(disk->cachemode);
+
+ virBufferVSprintf(&opt, ",cache=%s", mode);
+ } else if (disk->shared && !disk->readonly) {
+ virBufferAddLit(&opt, ",cache=off");
+ }
+
+ if (virBufferError(&opt)) {
+ virReportOOMError(conn);
+ goto error;
+ }
+
+ *str = virBufferContentAndReset(&opt);
+ return 0;
+
+error:
+ optstr = virBufferContentAndReset(&opt);
+ VIR_FREE(optstr);
+ *str = NULL;
+ return -1;
+}
+
+
int
qemuBuildNicStr(virConnectPtr conn,
virDomainNetDefPtr net,
@@ -1562,23 +1666,6 @@ static void qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev,
}
}
-#define QEMU_SERIAL_PARAM_ACCEPTED_CHARS \
- "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_"
-
-static int
-qemuSafeSerialParamValue(virConnectPtr conn,
- const char *value)
-{
- if (strspn(value, QEMU_SERIAL_PARAM_ACCEPTED_CHARS) != strlen (value)) {
- qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _("driver serial '%s' contains unsafe
characters"),
- value);
- return -1;
- }
-
- return 0;
-}
-
/*
* Constructs a argv suitable for launching qemu with config defined
* for a given virtual machine.
@@ -1960,12 +2047,9 @@ int qemudBuildCommandLine(virConnectPtr conn,
}
for (i = 0 ; i < def->ndisks ; i++) {
- virBuffer opt = VIR_BUFFER_INITIALIZER;
char *optstr;
int bootable = 0;
virDomainDiskDefPtr disk = def->disks[i];
- int idx = virDiskNameToIndex(disk->dst);
- const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
@@ -1980,12 +2064,6 @@ int qemudBuildCommandLine(virConnectPtr conn,
ADD_ARG_SPACE;
- if (idx < 0) {
- qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _("unsupported disk type '%s'"),
disk->dst);
- goto error;
- }
-
switch (disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
bootable = bootCD;
@@ -2001,64 +2079,8 @@ int qemudBuildCommandLine(virConnectPtr conn,
break;
}
- if (disk->src) {
- if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) {
- /* QEMU only supports magic FAT format for now */
- if (disk->driverType &&
- STRNEQ(disk->driverType, "fat")) {
- qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _("unsupported disk driver type for
'%s'"),
- disk->driverType);
- goto error;
- }
- if (!disk->readonly) {
- qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
"%s",
- _("cannot create virtual FAT disks in
read-write mode"));
- goto error;
- }
- if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
- virBufferVSprintf(&opt, "file=fat:floppy:%s,",
disk->src);
- else
- virBufferVSprintf(&opt, "file=fat:%s,",
disk->src);
- } else {
- virBufferVSprintf(&opt, "file=%s,", disk->src);
- }
- }
- virBufferVSprintf(&opt, "if=%s", bus);
- if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
- virBufferAddLit(&opt, ",media=cdrom");
- virBufferVSprintf(&opt, ",index=%d", idx);
- if (bootable &&
- disk->device == VIR_DOMAIN_DISK_DEVICE_DISK)
- virBufferAddLit(&opt, ",boot=on");
- if (disk->driverType &&
- disk->type != VIR_DOMAIN_DISK_TYPE_DIR &&
- qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_FORMAT)
- virBufferVSprintf(&opt, ",format=%s",
disk->driverType);
- if (disk->serial &&
- (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_SERIAL)) {
- if (qemuSafeSerialParamValue(conn, disk->serial) < 0)
- goto error;
- virBufferVSprintf(&opt, ",serial=%s", disk->serial);
- }
-
- if (disk->cachemode) {
- const char *mode =
- (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_CACHE_V2) ?
- qemuDiskCacheV2TypeToString(disk->cachemode) :
- qemuDiskCacheV1TypeToString(disk->cachemode);
-
- virBufferVSprintf(&opt, ",cache=%s", mode);
- } else if (disk->shared && !disk->readonly) {
- virBufferAddLit(&opt, ",cache=off");
- }
-
- if (virBufferError(&opt)) {
- virReportOOMError(conn);
+ if (qemuBuildDriveStr(conn, disk, bootable, qemuCmdFlags, &optstr) <
0)
goto error;
- }
-
- optstr = virBufferContentAndReset(&opt);
if ((qargv[qargc++] = strdup("-drive")) == NULL) {
VIR_FREE(optstr);
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 248677a..cd59d4c 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -191,6 +191,13 @@ int qemuBuildNicStr (virConnectPtr conn,
int vlan,
char **str);
+
+int qemuBuildDriveStr (virConnectPtr conn,
+ virDomainDiskDefPtr disk,
+ int bootable,
+ int qemuCmdFlags,
+ char **str);
+
int qemudNetworkIfaceConnect (virConnectPtr conn,
struct qemud_driver *driver,
virDomainNetDefPtr net,
--
1.6.5.2
4:22 p.m.
New subject: [libvirt] [PATCH 09/12] Specify bus/unit instead of index for disks with QEMU
The current code for using -drive simply sets the -drive 'index'
parameter. QEMU internally converts this to bus/unit depending
on the type of drive. This does not give us precise control over
the bus/unit assignment though. This change switches over to make
libvirt explicitly calculate the bus/unit number.
In addition bus/unit/index are actually irrelevant for VirtIO
disks, since each virtio disk is a separate PCI device. No disk
controller is involved.
Doing the conversion to bus/unit in libvirt allows us to correctly
attach SCSI controllers when required.
* src/qemu/qemu_conf.c: Specify bus/unit instead of index for
disks
---
src/qemu/qemu_conf.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 79 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 27faa3f..f4fbe96 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1342,6 +1342,7 @@ qemuBuildDriveStr(virConnectPtr conn,
const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
int idx = virDiskNameToIndex(disk->dst);
char *optstr = NULL;
+ int controllerid = -1, busid = -1, unitid = -1;
if (idx < 0) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
@@ -1349,6 +1350,80 @@ qemuBuildDriveStr(virConnectPtr conn,
goto error;
}
+ switch (disk->bus) {
+ case VIR_DOMAIN_DISK_BUS_SCSI:
+ case VIR_DOMAIN_DISK_BUS_IDE:
+ case VIR_DOMAIN_DISK_BUS_FDC:
+ if (disk->addr.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE &&
+ disk->addr.mode == VIR_DOMAIN_DEVICE_ADDRESS_MODE_STATIC) {
+ /* The user specified an explicit address, so ignore dev name */
+ controllerid = disk->addr.data.drive.controller;
+ busid = disk->addr.data.drive.bus;
+ unitid = disk->addr.data.drive.unit;
+ } else if (disk->addr.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
+ disk->addr.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
+ /*
+ * We auto-assign contoller/bus/unit based on the drive
+ * index, which was in turn based on dev name eg "sda"
+ */
+ if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
+ /* Many SCSI controllers, only 1 bus, each with 7 units */
+ controllerid = idx / 7;
+ busid = 0;
+ unitid = idx % 7;
+ } else if (disk->bus == VIR_DOMAIN_DISK_BUS_IDE) {
+ /* One IDE controller, with 2 buses, each 2 units */
+ controllerid = 0;
+ busid = idx / 2;
+ unitid = idx % 2;
+ } else {
+ controllerid = 0;
+ busid = 0;
+ unitid = idx;
+ }
+
+ disk->addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
+ disk->addr.mode = VIR_DOMAIN_DEVICE_ADDRESS_MODE_DYNAMIC;
+ disk->addr.data.drive.controller = controllerid;
+ disk->addr.data.drive.bus = busid;
+ disk->addr.data.drive.unit = unitid;
+ } else {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s",
+ _("unexpected address type for ide/scsi/fdc
disk"));
+ goto error;
+ }
+ break;
+
+ case VIR_DOMAIN_DISK_BUS_VIRTIO:
+ /* Each virtio drive is a separate PCI device, no unit/busid */
+ break;
+
+ case VIR_DOMAIN_DISK_BUS_XEN:
+ /* Xen has no address type currently, so assign based on index */
+ unitid = idx;
+ break;
+ }
+
+ if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
+ if (busid != 0) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("SCSI controller only supports 1 bus"));
+ goto error;
+ }
+ /* Setting bus= attr for SCSI drives, causes a controller
+ * to be created. Yes this is slightly odd. It is not possible
+ * to have > 1 bus on a SCSI controller (yet). */
+ busid = controllerid;
+ } else if (disk->bus == VIR_DOMAIN_DISK_BUS_IDE ||
+ disk->bus == VIR_DOMAIN_DISK_BUS_FDC) {
+ /* We can only have 1 IDE / Floppy controller (currently) */
+ if (controllerid != 0) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("Only 1 %s controller is supported"), bus);
+ goto error;
+ }
+ }
+
if (disk->src) {
if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) {
/* QEMU only supports magic FAT format for now */
@@ -1375,7 +1450,10 @@ qemuBuildDriveStr(virConnectPtr conn,
virBufferVSprintf(&opt, "if=%s", bus);
if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
virBufferAddLit(&opt, ",media=cdrom");
- virBufferVSprintf(&opt, ",index=%d", idx);
+ if (busid != -1)
+ virBufferVSprintf(&opt, ",bus=%d", busid);
+ if (unitid != -1)
+ virBufferVSprintf(&opt, ",unit=%d", unitid);
if (bootable &&
disk->device == VIR_DOMAIN_DISK_DEVICE_DISK)
virBufferAddLit(&opt, ",boot=on");
--
1.6.5.2
4:22 p.m.
New subject: [libvirt] [PATCH 10/12] Implement SCSI controller hotplug/unplug for QEMU
From: Wolfgang Mauerer <wolfgang.mauerer(a)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
---
src/libvirt_private.syms | 2 +
src/qemu/qemu_driver.c | 117 ++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor.c | 16 ++++++
src/qemu/qemu_monitor.h | 3 +
src/qemu/qemu_monitor_json.c | 40 ++++++++++++++
src/qemu/qemu_monitor_json.h | 4 ++
src/qemu/qemu_monitor_text.c | 43 +++++++++++++++
src/qemu/qemu_monitor_text.h | 5 ++
8 files changed, 230 insertions(+), 0 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8622e5b..3e98800 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -165,6 +165,8 @@ virDomainDeviceUSBAddressIsValid;
virDomainDeviceAddressClear;
virDomainDeviceAddressTypeToString;
virDomainDefClearDynamicValues;
+virDomainControllerTypeToString;
+virDomainControllerDefFree;
# domain_event.h
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 321cd4e..0a91c2a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4961,6 +4961,44 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
return ret;
}
+static int qemudDomainAttachPciControllerDevice(virConnectPtr conn,
+ struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
+{
+ int i, ret;
+ const char* type =
virDomainControllerTypeToString(dev->data.controller->type);
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ for (i = 0 ; i < vm->def->ncontrollers ; i++) {
+ if ((vm->def->controllers[i]->type == dev->data.controller->type)
&&
+ (vm->def->controllers[i]->idx == dev->data.controller->idx))
{
+ qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ _("target %s:%d already exists"),
+ type, dev->data.controller->idx);
+ return -1;
+ }
+ }
+
+ if (VIR_REALLOC_N(vm->def->controllers, vm->def->ncontrollers+1) < 0)
{
+ virReportOOMError(conn);
+ return -1;
+ }
+
+ qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ ret = qemuMonitorAttachPCIDiskController(priv->mon,
+ type,
+
&dev->data.controller->addr.data.pci);
+ qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+ if (ret == 0) {
+ dev->data.controller->addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+ virDomainControllerInsertPreAlloced(vm->def, dev->data.controller);
+ }
+
+ return ret;
+}
+
static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr conn,
struct qemud_driver *driver,
virDomainObjPtr vm,
@@ -5345,6 +5383,15 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
virCgroupDenyDevicePath(cgroup,
dev->data.disk->src);
}
+ } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) {
+ if (dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
+ ret = qemudDomainAttachPciControllerDevice(dom->conn, driver, vm, dev);
+ } else {
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
+ _("disk controller bus '%s' cannot be
hotplugged."),
+
virDomainControllerTypeToString(dev->data.controller->type));
+ /* fallthrough */
+ }
} else if (dev->type == VIR_DOMAIN_DEVICE_NET) {
ret = qemudDomainAttachNetDevice(dom->conn, driver, vm, dev, qemuCmdFlags);
} else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) {
@@ -5436,6 +5483,67 @@ cleanup:
return ret;
}
+static int qemudDomainDetachPciControllerDevice(virConnectPtr conn,
+ struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
+{
+ int i, ret = -1;
+ virDomainControllerDefPtr detach = NULL;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ for (i = 0 ; i < vm->def->ncontrollers ; i++) {
+ if ((vm->def->controllers[i]->type == dev->data.controller->type)
&&
+ (vm->def->controllers[i]->idx == dev->data.controller->idx))
{
+ detach = vm->def->controllers[i];
+ break;
+ }
+ }
+
+ if (!detach) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ _("disk controller %s:%d not found"),
+
virDomainControllerTypeToString(dev->data.controller->type),
+ dev->data.controller->idx);
+ goto cleanup;
+ }
+
+ if (!virDomainDeviceAddressIsValid(&detach->addr,
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, "%s",
+ _("device cannot be detached without a PCI
address"));
+ goto cleanup;
+ }
+
+ qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ if (qemuMonitorRemovePCIDevice(priv->mon,
+ &detach->addr.data.pci) < 0) {
+ qemuDomainObjExitMonitor(vm);
+ goto cleanup;
+ }
+ qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+ if (vm->def->ncontrollers > 1) {
+ memmove(vm->def->controllers + i,
+ vm->def->controllers + i + 1,
+ sizeof(*vm->def->controllers) *
+ (vm->def->ncontrollers - (i + 1)));
+ vm->def->ncontrollers--;
+ if (VIR_REALLOC_N(vm->def->controllers, vm->def->ncontrollers) <
0) {
+ /* ignore, harmless */
+ }
+ } else {
+ VIR_FREE(vm->def->controllers);
+ vm->def->ncontrollers = 0;
+ }
+ virDomainControllerDefFree(detach);
+
+ ret = 0;
+
+cleanup:
+ return ret;
+}
+
static int
qemudDomainDetachNetDevice(virConnectPtr conn,
struct qemud_driver *driver,
@@ -5687,6 +5795,15 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
VIR_WARN0("Fail to restore disk device ownership");
} else if (dev->type == VIR_DOMAIN_DEVICE_NET) {
ret = qemudDomainDetachNetDevice(dom->conn, driver, vm, dev);
+ } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) {
+ if (dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
+ ret = qemudDomainDetachPciControllerDevice(dom->conn, driver, vm, dev);
+ } else {
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
+ _("disk controller bus '%s' cannot be
hotunplugged."),
+
virDomainControllerTypeToString(dev->data.controller->type));
+ /* fallthrough */
+ }
} else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) {
ret = qemudDomainDetachHostDevice(dom->conn, driver, vm, dev);
} else
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ded1622..4ecbcba 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1240,3 +1240,19 @@ int qemuMonitorRemoveHostNetwork(qemuMonitorPtr mon,
ret = qemuMonitorTextRemoveHostNetwork(mon, vlan, netname);
return ret;
}
+
+
+int qemuMonitorAttachPCIDiskController(qemuMonitorPtr mon,
+ const char *bus,
+ virDomainDevicePCIAddress *guestAddr)
+{
+ DEBUG("mon=%p, fd=%d type=%s", mon, mon->fd, bus);
+ int ret;
+
+ if (mon->json)
+ ret = qemuMonitorJSONAttachPCIDiskController(mon, bus, guestAddr);
+ else
+ ret = qemuMonitorTextAttachPCIDiskController(mon, bus, guestAddr);
+
+ return ret;
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 46f412e..70827c7 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -260,5 +260,8 @@ int qemuMonitorRemoveHostNetwork(qemuMonitorPtr mon,
int vlan,
const char *netname);
+int qemuMonitorAttachPCIDiskController(qemuMonitorPtr mon,
+ const char *bus,
+ virDomainDevicePCIAddress *guestAddr);
#endif /* QEMU_MONITOR_H */
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 48eca13..2e605ac 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1481,3 +1481,43 @@ int qemuMonitorJSONRemoveHostNetwork(qemuMonitorPtr mon,
virJSONValueFree(reply);
return ret;
}
+
+
+int qemuMonitorJSONAttachPCIDiskController(qemuMonitorPtr mon,
+ const char *bus,
+ virDomainDevicePCIAddress *guestAddr)
+{
+ int ret;
+ virJSONValuePtr cmd;
+ virJSONValuePtr reply = NULL;
+ char *dev;
+
+ memset(guestAddr, 0, sizeof(*guestAddr));
+
+ if (virAsprintf(&dev, "if=%s", bus) < 0) {
+ virReportOOMError(NULL);
+ return -1;
+ }
+
+ cmd = qemuMonitorJSONMakeCommand("pci_add",
+ "s:pci_addr", "auto",
+ "s:type", "storage",
+ "s:opts", dev,
+ NULL);
+ VIR_FREE(dev);
+ if (!cmd)
+ return -1;
+
+ ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+
+ if (ret == 0)
+ ret = qemuMonitorJSONCheckError(cmd, reply);
+
+ if (ret == 0 &&
+ qemuMonitorJSONGetGuestAddress(reply, guestAddr) < 0)
+ ret = -1;
+
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+ return ret;
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 9df96f5..221dfa7 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -140,4 +140,8 @@ int qemuMonitorJSONRemoveHostNetwork(qemuMonitorPtr mon,
int vlan,
const char *netname);
+int qemuMonitorJSONAttachPCIDiskController(qemuMonitorPtr mon,
+ const char *bus,
+ virDomainDevicePCIAddress *guestAddr);
+
#endif /* QEMU_MONITOR_JSON_H */
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 27c213d..a9d8a4c 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -1577,3 +1577,46 @@ cleanup:
VIR_FREE(reply);
return ret;
}
+
+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;
+ }
+
+ if (qemuMonitorCommand(mon, cmd, &reply) < 0) {
+ qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ _("cannot attach %s disk controller"), bus);
+ goto cleanup;
+ }
+
+ if (qemuMonitorTextParsePciAddReply(mon, reply, guestAddr) < 0) {
+ if (!tryOldSyntax && strstr(reply, "invalid char in
expression")) {
+ VIR_FREE(reply);
+ VIR_FREE(cmd);
+ tryOldSyntax = 1;
+ goto try_command;
+ }
+
+ qemudReportError (NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ _("adding %s disk controller failed: %s"), bus,
reply);
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(cmd);
+ VIR_FREE(reply);
+ return ret;
+}
diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h
index f304795..0e131ea 100644
--- a/src/qemu/qemu_monitor_text.h
+++ b/src/qemu/qemu_monitor_text.h
@@ -140,4 +140,9 @@ int qemuMonitorTextRemoveHostNetwork(qemuMonitorPtr mon,
int vlan,
const char *netname);
+int qemuMonitorTextAttachPCIDiskController(qemuMonitorPtr mon,
+ const char *bus,
+ virDomainDevicePCIAddress *guestAddr);
+
+
#endif /* QEMU_MONITOR_TEXT_H */
--
1.6.5.2
11:30 a.m.
New subject: [libvirt] Re: [PATCH 10/12] Implement SCSI controller hotplug/unplug for QEMU
Hi,
Daniel P. Berrange wrote:
From: Wolfgang Mauerer <wolfgang.mauerer(a)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
11:38 a.m.
New subject: [libvirt] Re: [PATCH 10/12] Implement SCSI controller hotplug/unplug for QEMU
On Tue, Dec 15, 2009 at 06:30:30PM +0100, Wolfgang Mauerer wrote:
Hi,
Daniel P. Berrange wrote:
> From: Wolfgang Mauerer <wolfgang.mauerer(a)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.
I posted a patch to QEMU last week for that too and they accepted it
but it hasn't been merged for some reason
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 :|
11:40 a.m.
New subject: [libvirt] Re: [PATCH 10/12] Implement SCSI controller hotplug/unplug for QEMU
On Tue, Dec 15, 2009 at 06:30:30PM +0100, Wolfgang Mauerer wrote:
Hi,
Daniel P. Berrange wrote:
> From: Wolfgang Mauerer <wolfgang.mauerer(a)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.
Oh I should mention too that with new QEMU 0.12, we can add controllers
in a different way
-device lsi
Or in the monitor
device_add lsi
My other series of patches converts everything over to use -device for
new enough QEMU, so I'll extend that to cover controllers anyway. The
added advantage of -device is that we can set PCI addresses at startup
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 :|
4:22 p.m.
New subject: [libvirt] [PATCH 11/12] Properly support SCSI drive hotplug
The current SCSI hotplug support attaches a brand new SCSI controller
for every disk. This is broken because the semantics differ from those
used when starting the VM initially. In the latter case, each SCSI
controller is filled before a new one is added.
If the user specifies an high drive index (sdazz) then at initial
startup, many intermediate SCSI controllers may be added with no
drives.
This patch changes SCSI hotplug so that it exactly matches the
behaviour of initial startup. First the SCSI controller number is
determined for the drive to be hotplugged. If any controller upto
and including that controller number is not yet present, it is
attached. Then finally the drive is attached to the last controller.
NB, this breaks SCSI hotunplug, because there is no 'drive_del'
command in current QEMU. Previous SCSI hotunplug was broken in
any case because it was unplugging the entire controller, not
just the drive in question.
A future QEMU will allow proper SCSI hotunplug of a drive.
This patch is derived from work done by Wolfgang Mauerer on disk
controllers.
* src/qemu/qemu_driver.c: Fix SCSI hotplug to add a drive to
the correct controller, instead of just attaching a new
controller.
* 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
support for 'drive_add' command
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 145 +++++++++++++++++++++++++++++++++++++----
src/qemu/qemu_monitor.c | 20 ++++++
src/qemu/qemu_monitor.h | 5 ++
src/qemu/qemu_monitor_json.c | 81 +++++++++++++++++++++--
src/qemu/qemu_monitor_json.h | 5 ++
src/qemu/qemu_monitor_text.c | 102 +++++++++++++++++++++++++++++
src/qemu/qemu_monitor_text.h | 5 ++
8 files changed, 342 insertions(+), 22 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3e98800..963f9b2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -167,6 +167,7 @@ virDomainDeviceAddressTypeToString;
virDomainDefClearDynamicValues;
virDomainControllerTypeToString;
virDomainControllerDefFree;
+virDomainDeviceAddressTypeToString;
# domain_event.h
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0a91c2a..527b5d7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4964,18 +4964,18 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
static int qemudDomainAttachPciControllerDevice(virConnectPtr conn,
struct qemud_driver *driver,
virDomainObjPtr vm,
- virDomainDeviceDefPtr dev)
+ virDomainControllerDefPtr dev)
{
int i, ret;
- const char* type =
virDomainControllerTypeToString(dev->data.controller->type);
+ const char* type = virDomainControllerTypeToString(dev->type);
qemuDomainObjPrivatePtr priv = vm->privateData;
for (i = 0 ; i < vm->def->ncontrollers ; i++) {
- if ((vm->def->controllers[i]->type == dev->data.controller->type)
&&
- (vm->def->controllers[i]->idx == dev->data.controller->idx))
{
+ if ((vm->def->controllers[i]->type == dev->type) &&
+ (vm->def->controllers[i]->idx == dev->idx)) {
qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
_("target %s:%d already exists"),
- type, dev->data.controller->idx);
+ type, dev->idx);
return -1;
}
}
@@ -4988,17 +4988,130 @@ static int qemudDomainAttachPciControllerDevice(virConnectPtr
conn,
qemuDomainObjEnterMonitorWithDriver(driver, vm);
ret = qemuMonitorAttachPCIDiskController(priv->mon,
type,
-
&dev->data.controller->addr.data.pci);
+ &dev->addr.data.pci);
qemuDomainObjExitMonitorWithDriver(driver, vm);
if (ret == 0) {
- dev->data.controller->addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
- virDomainControllerInsertPreAlloced(vm->def, dev->data.controller);
+ dev->addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+ virDomainControllerInsertPreAlloced(vm->def, dev);
}
return ret;
}
+static virDomainControllerDefPtr
+qemuDomainFindOrCreateSCSIDiskController(virConnectPtr conn,
+ struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ int controller)
+{
+ int i;
+ virDomainControllerDefPtr cont;
+ for (i = 0 ; i < vm->def->ncontrollers ; i++) {
+ cont = vm->def->controllers[i];
+
+ if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
+ continue;
+
+ if (cont->idx == controller)
+ return cont;
+ }
+
+ /* No SCSI controller present, for back compatability we
+ * now hotplug a controller */
+ if (VIR_ALLOC(cont) < 0) {
+ virReportOOMError(conn);
+ return NULL;
+ }
+ cont->type = VIR_DOMAIN_CONTROLLER_TYPE_SCSI;
+ cont->idx = 0;
+
+ VIR_INFO0("No SCSI controller present, hotplugging one");
+ if (qemudDomainAttachPciControllerDevice(conn, driver,
+ vm, cont) < 0) {
+ VIR_FREE(cont);
+ return NULL;
+ }
+ return cont;
+}
+
+static int qemudDomainAttachSCSIDisk(virConnectPtr conn,
+ struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr dev,
+ int qemuCmdFlags)
+{
+ int i;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virDomainDeviceDriveAddress driveAddr;
+ virDomainControllerDefPtr cont;
+ char *drivestr = NULL;
+ int ret = -1;
+
+ for (i = 0 ; i < vm->def->ndisks ; i++) {
+ if (STREQ(vm->def->disks[i]->dst, dev->dst)) {
+ qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ _("target %s already exists"), dev->dst);
+ goto cleanup;
+ }
+ }
+
+ /* This func allocates the bus/unit IDs so must be before
+ * we search for controller
+ */
+ if (qemuBuildDriveStr(conn, dev, 0, qemuCmdFlags, &drivestr) < 0)
+ goto cleanup;
+
+
+ /* We should have an adddress now, so make sure */
+ if (dev->addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("unexpected disk address type %s"),
+ virDomainDeviceAddressTypeToString(dev->addr.type));
+ goto cleanup;
+ }
+
+ for (i = 0 ; i < dev->addr.data.drive.controller ; i++) {
+ cont = qemuDomainFindOrCreateSCSIDiskController(conn, driver, vm, i);
+ if (!cont)
+ goto cleanup;
+ }
+
+ if (cont->addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("SCSI controller %d was missing its PCI address"),
cont->idx);
+ goto cleanup;
+ }
+
+ if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) {
+ virReportOOMError(conn);
+ goto cleanup;
+ }
+
+ qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ ret = qemuMonitorAttachDrive(priv->mon,
+ drivestr,
+ &cont->addr.data.pci,
+ &driveAddr);
+
+ qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+ if (ret == 0) {
+ if (dev->addr.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE ||
+ dev->addr.mode == VIR_DOMAIN_DEVICE_ADDRESS_MODE_DYNAMIC) {
+ dev->addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
+ dev->addr.mode = VIR_DOMAIN_DEVICE_ADDRESS_MODE_DYNAMIC;
+ memcpy(&dev->addr.data.drive, &driveAddr, sizeof(driveAddr));
+ }
+ virDomainDiskInsertPreAlloced(vm->def, dev);
+ }
+
+cleanup:
+ VIR_FREE(drivestr);
+ return ret;
+}
+
+
static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr conn,
struct qemud_driver *driver,
virDomainObjPtr vm,
@@ -5362,9 +5475,10 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
ret = qemudDomainAttachUsbMassstorageDevice(dom->conn, driver, vm,
dev);
- } else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI ||
- dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
+ } else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
ret = qemudDomainAttachPciDiskDevice(dom->conn, driver, vm, dev);
+ } else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
+ ret = qemudDomainAttachSCSIDisk(dom->conn, driver, vm,
dev->data.disk, qemuCmdFlags);
} else {
qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
_("disk bus '%s' cannot be
hotplugged."),
@@ -5385,7 +5499,8 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
}
} else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) {
if (dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
- ret = qemudDomainAttachPciControllerDevice(dom->conn, driver, vm, dev);
+ ret = qemudDomainAttachPciControllerDevice(dom->conn, driver, vm,
+ dev->data.controller);
} else {
qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
_("disk controller bus '%s' cannot be
hotplugged."),
@@ -5786,8 +5901,7 @@ 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_SCSI ||
- dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO)) {
+ dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
ret = qemudDomainDetachPciDiskDevice(dom->conn, driver, vm, dev);
if (driver->securityDriver)
driver->securityDriver->domainRestoreSecurityImageLabel(dom->conn,
vm, dev->data.disk);
@@ -5806,9 +5920,10 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
}
} else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) {
ret = qemudDomainDetachHostDevice(dom->conn, driver, vm, dev);
- } else
+ } else {
qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
- "%s", _("only SCSI or virtio disk device can be
detached dynamically"));
+ "%s", _("This type of device cannot be hot
unplugged"));
+ }
if (!ret && virDomainSaveStatus(dom->conn, driver->caps,
driver->stateDir, vm) < 0)
ret = -1;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 4ecbcba..30f2cce 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1256,3 +1256,23 @@ int qemuMonitorAttachPCIDiskController(qemuMonitorPtr mon,
return ret;
}
+
+
+int qemuMonitorAttachDrive(qemuMonitorPtr mon,
+ const char *drivestr,
+ virDomainDevicePCIAddress *controllerAddr,
+ virDomainDeviceDriveAddress *driveAddr)
+{
+ DEBUG("mon=%p, fd=%d drivestr=%s domain=%d bus=%d slot=%d function=%d",
+ mon, mon->fd, drivestr,
+ controllerAddr->domain, controllerAddr->bus,
+ controllerAddr->slot, controllerAddr->function);
+ int ret;
+
+ if (mon->json)
+ ret = qemuMonitorJSONAttachDrive(mon, drivestr, controllerAddr, driveAddr);
+ else
+ ret = qemuMonitorTextAttachDrive(mon, drivestr, controllerAddr, driveAddr);
+
+ return ret;
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 70827c7..fcfdcd5 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -264,4 +264,9 @@ int qemuMonitorAttachPCIDiskController(qemuMonitorPtr mon,
const char *bus,
virDomainDevicePCIAddress *guestAddr);
+int qemuMonitorAttachDrive(qemuMonitorPtr mon,
+ const char *drivestr,
+ virDomainDevicePCIAddress *controllerAddr,
+ virDomainDeviceDriveAddress *driveAddr);
+
#endif /* QEMU_MONITOR_H */
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 2e605ac..61df373 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1202,10 +1202,9 @@ int qemuMonitorJSONAddUSBDeviceMatch(qemuMonitorPtr mon,
}
-/* XXX qemu also returns a 'function' number now */
static int
-qemuMonitorJSONGetGuestAddress(virJSONValuePtr reply,
- virDomainDevicePCIAddress *guestAddr)
+qemuMonitorJSONGetGuestPCIAddress(virJSONValuePtr reply,
+ virDomainDevicePCIAddress *guestAddr)
{
virJSONValuePtr addr;
@@ -1277,7 +1276,7 @@ int qemuMonitorJSONAddPCIHostDevice(qemuMonitorPtr mon,
ret = qemuMonitorJSONCheckError(cmd, reply);
if (ret == 0 &&
- qemuMonitorJSONGetGuestAddress(reply, guestAddr) < 0)
+ qemuMonitorJSONGetGuestPCIAddress(reply, guestAddr) < 0)
ret = -1;
virJSONValueFree(cmd);
@@ -1318,7 +1317,7 @@ int qemuMonitorJSONAddPCIDisk(qemuMonitorPtr mon,
ret = qemuMonitorJSONCheckError(cmd, reply);
if (ret == 0 &&
- qemuMonitorJSONGetGuestAddress(reply, guestAddr) < 0)
+ qemuMonitorJSONGetGuestPCIAddress(reply, guestAddr) < 0)
ret = -1;
virJSONValueFree(cmd);
@@ -1350,7 +1349,7 @@ int qemuMonitorJSONAddPCINetwork(qemuMonitorPtr mon,
ret = qemuMonitorJSONCheckError(cmd, reply);
if (ret == 0 &&
- qemuMonitorJSONGetGuestAddress(reply, guestAddr) < 0)
+ qemuMonitorJSONGetGuestPCIAddress(reply, guestAddr) < 0)
ret = -1;
virJSONValueFree(cmd);
@@ -1514,7 +1513,75 @@ int qemuMonitorJSONAttachPCIDiskController(qemuMonitorPtr mon,
ret = qemuMonitorJSONCheckError(cmd, reply);
if (ret == 0 &&
- qemuMonitorJSONGetGuestAddress(reply, guestAddr) < 0)
+ qemuMonitorJSONGetGuestPCIAddress(reply, guestAddr) < 0)
+ ret = -1;
+
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+ return ret;
+}
+
+
+static int
+qemuMonitorJSONGetGuestDriveAddress(virJSONValuePtr reply,
+ virDomainDeviceDriveAddress *driveAddr)
+{
+ virJSONValuePtr addr;
+
+ addr = virJSONValueObjectGet(reply, "return");
+ if (!addr || addr->type != VIR_JSON_TYPE_OBJECT) {
+ qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("drive_add reply was missing device address"));
+ return -1;
+ }
+
+ if (virJSONValueObjectGetNumberUint(addr, "bus", &driveAddr->bus)
< 0) {
+ qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("drive_add reply was missing device bus number"));
+ return -1;
+ }
+
+ if (virJSONValueObjectGetNumberUint(addr, "unit", &driveAddr->unit)
< 0) {
+ qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("drive_add reply was missing device unit number"));
+ return -1;
+ }
+
+ return 0;
+}
+
+
+int qemuMonitorJSONAttachDrive(qemuMonitorPtr mon,
+ const char *drivestr,
+ virDomainDevicePCIAddress* controllerAddr,
+ virDomainDeviceDriveAddress* driveAddr)
+{
+ int ret;
+ virJSONValuePtr cmd = NULL;
+ virJSONValuePtr reply = NULL;
+ char *dev;
+
+ if (virAsprintf(&dev, "%.2x:%.2x.%.1x",
+ controllerAddr->bus, controllerAddr->slot,
controllerAddr->function) < 0) {
+ virReportOOMError(NULL);
+ return -1;
+ }
+
+ cmd = qemuMonitorJSONMakeCommand("drive_add",
+ "s:pci_addr", dev,
+ "s:opts", drivestr,
+ NULL);
+ VIR_FREE(dev);
+ if (!cmd)
+ return -1;
+
+ ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+
+ if (ret == 0)
+ ret = qemuMonitorJSONCheckError(cmd, reply);
+
+ if (ret == 0 &&
+ qemuMonitorJSONGetGuestDriveAddress(reply, driveAddr) < 0)
ret = -1;
virJSONValueFree(cmd);
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 221dfa7..1997dbb 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -144,4 +144,9 @@ int qemuMonitorJSONAttachPCIDiskController(qemuMonitorPtr mon,
const char *bus,
virDomainDevicePCIAddress *guestAddr);
+int qemuMonitorJSONAttachDrive(qemuMonitorPtr mon,
+ const char *drivestr,
+ virDomainDevicePCIAddress *controllerAddr,
+ virDomainDeviceDriveAddress *driveAddr);
+
#endif /* QEMU_MONITOR_JSON_H */
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index a9d8a4c..16bc81f 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -1620,3 +1620,105 @@ cleanup:
VIR_FREE(reply);
return ret;
}
+
+
+static int
+qemudParseDriveAddReply(const char *reply,
+ virDomainDeviceDriveAddressPtr addr)
+{
+ char *s, *e;
+
+ /* 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_ui(s, &e, 10, &addr->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_ui(s, &e, 10, &addr->unit) == -1) {
+ VIR_WARN(_("Unable to parse unit number '%s'\n"), s);
+ return -1;
+ }
+
+ return 0;
+}
+
+
+int qemuMonitorTextAttachDrive(qemuMonitorPtr mon,
+ const char *drivestr,
+ virDomainDevicePCIAddress *controllerAddr,
+ virDomainDeviceDriveAddress *driveAddr)
+{
+ char *cmd = NULL;
+ char *reply = NULL;
+ int ret = -1;
+ char *safe_str;
+ int tryOldSyntax = 0;
+
+ safe_str = qemuMonitorEscapeArg(drivestr);
+ if (!safe_str) {
+ virReportOOMError(NULL);
+ return -1;
+ }
+
+try_command:
+ ret = virAsprintf(&cmd, "drive_add %s%.2x:%.2x:%.2x %s",
+ (tryOldSyntax ? "" : "pci_addr="),
+ controllerAddr->domain, controllerAddr->bus,
+ controllerAddr->slot, safe_str);
+ if (ret == -1) {
+ virReportOOMError(NULL);
+ goto cleanup;
+ }
+
+ if (qemuMonitorCommand(mon, cmd, &reply) < 0) {
+ qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ _("failed to close fd in qemu with '%s'"),
cmd);
+ goto cleanup;
+ }
+
+ if (qemudParseDriveAddReply(reply, driveAddr) < 0) {
+ if (!tryOldSyntax && strstr(reply, "invalid char in
expression")) {
+ VIR_FREE(reply);
+ tryOldSyntax = 1;
+ goto try_command;
+ }
+ qemudReportError (NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ _("adding %s disk failed: %s"), drivestr, reply);
+ VIR_FREE(reply);
+ return -1;
+ }
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(cmd);
+ VIR_FREE(reply);
+ VIR_FREE(safe_str);
+ return ret;
+}
+
+
diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h
index 0e131ea..94fcc02 100644
--- a/src/qemu/qemu_monitor_text.h
+++ b/src/qemu/qemu_monitor_text.h
@@ -144,5 +144,10 @@ int qemuMonitorTextAttachPCIDiskController(qemuMonitorPtr mon,
const char *bus,
virDomainDevicePCIAddress *guestAddr);
+int qemuMonitorTextAttachDrive(qemuMonitorPtr mon,
+ const char *drivestr,
+ virDomainDevicePCIAddress *controllerAddr,
+ virDomainDeviceDriveAddress *driveAddr);
+
#endif /* QEMU_MONITOR_TEXT_H */
--
1.6.5.2
4:22 p.m.
New subject: [libvirt] [PATCH 12/12] Detect PCI addresses at QEMU startup
Hotunplug of devices requires that we know their PCI address. Even
hotplug of SCSI drives, required that we know the PCI address of
the SCSI controller to attach the drive to. We can find this out
by running 'info pci' and then correlating the vendor/product IDs
with the devices we booted with.
* src/qemu/qemu_driver.c: Assign all dynamic PCI addresses on
startup of QEMU VM, matching vendor/product IDs
* 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
API for fetching PCI device address mapping
---
src/qemu/qemu_driver.c | 365 ++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor.c | 13 ++
src/qemu/qemu_monitor.h | 11 ++
src/qemu/qemu_monitor_json.c | 7 +
src/qemu/qemu_monitor_json.h | 3 +
src/qemu/qemu_monitor_text.c | 129 +++++++++++++++
src/qemu/qemu_monitor_text.h | 2 +
7 files changed, 530 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 527b5d7..999062d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1598,6 +1598,368 @@ qemuInitPasswords(struct qemud_driver *driver,
}
+#define QEMU_PCI_VENDOR_INTEL 0x8086
+#define QEMU_PCI_VENDOR_LSI_LOGIC 0x1000
+#define QEMU_PCI_VENDOR_REDHAT 0x1af4
+#define QEMU_PCI_VENDOR_CIRRUS 0x1013
+#define QEMU_PCI_VENDOR_REALTEK 0x10ec
+#define QEMU_PCI_VENDOR_AMD 0x1022
+#define QEMU_PCI_VENDOR_ENSONIQ 0x1274
+#define QEMU_PCI_VENDOR_VMWARE 0x15ad
+#define QEMU_PCI_VENDOR_QEMU 0x1234
+
+#define QEMU_PCI_PRODUCT_DISK_VIRTIO 0x1001
+
+#define QEMU_PCI_PRODUCT_NIC_NE2K 0x8029
+#define QEMU_PCI_PRODUCT_NIC_PCNET 0x2000
+#define QEMU_PCI_PRODUCT_NIC_RTL8139 0x8139
+#define QEMU_PCI_PRODUCT_NIC_E1000 0x100E
+#define QEMU_PCI_PRODUCT_NIC_VIRTIO 0x1000
+
+#define QEMU_PCI_PRODUCT_VGA_CIRRUS 0x00b8
+#define QEMU_PCI_PRODUCT_VGA_VMWARE 0x0405
+#define QEMU_PCI_PRODUCT_VGA_STDVGA 0x1111
+
+#define QEMU_PCI_PRODUCT_AUDIO_AC97 0x2415
+#define QEMU_PCI_PRODUCT_AUDIO_ES1370 0x5000
+
+#define QEMU_PCI_PRODUCT_CONTROLLER_PIIX 0x7010
+#define QEMU_PCI_PRODUCT_CONTROLLER_LSI 0x0012
+
+#define QEMU_PCI_PRODUCT_WATCHDOG_I63000ESB 0x25ab
+
+static int
+qemuAssignNextPCIAddress(virDomainDeviceAddress *addr,
+ int vendor,
+ int product,
+ qemuMonitorPCIAddress *addrs,
+ int naddrs)
+{
+ int found = 0;
+ int i;
+
+ VIR_DEBUG("Look for %x:%x out of %d", vendor, product, naddrs);
+
+ for (i = 0 ; (i < naddrs) && !found; i++) {
+ VIR_DEBUG("Maybe %x:%x", addrs[i].vendor, addrs[i].product);
+ if (addrs[i].vendor == vendor &&
+ addrs[i].product == product) {
+ VIR_DEBUG("Match %d", i);
+ found = 1;
+ break;
+ }
+ }
+ if (!found) {
+ return -1;
+ }
+
+ /* Blank it out so this device isn't matched again */
+ addrs[i].vendor = 0;
+ addrs[i].product = 0;
+
+ if (addr->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+ addr->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+
+ if (addr->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+ if (addr->mode == VIR_DOMAIN_DEVICE_ADDRESS_MODE_DYNAMIC) {
+ addr->data.pci.domain = addrs[i].addr.domain;
+ addr->data.pci.bus = addrs[i].addr.bus;
+ addr->data.pci.slot = addrs[i].addr.slot;
+ addr->data.pci.function = addrs[i].addr.function;
+ } else {
+ /* XXX should we validate the device addr matches
+ * the static addr we requested ? */
+ }
+ }
+
+ return 0;
+}
+
+static int
+qemuGetPCIDiskVendorProduct(virDomainDiskDefPtr def,
+ unsigned *vendor,
+ unsigned *product)
+{
+ switch (def->bus) {
+ case VIR_DOMAIN_DISK_BUS_VIRTIO:
+ *vendor = QEMU_PCI_VENDOR_REDHAT;
+ *product = QEMU_PCI_PRODUCT_DISK_VIRTIO;
+ break;
+
+ default:
+ return -1;
+ }
+
+ return 0;
+}
+
+static int
+qemuGetPCINetVendorProduct(virDomainNetDefPtr def,
+ unsigned *vendor,
+ unsigned *product)
+{
+ if (!def->model)
+ return -1;
+
+ if (STREQ(def->model, "ne2k_pci")) {
+ *vendor = QEMU_PCI_VENDOR_REALTEK;
+ *product = QEMU_PCI_PRODUCT_NIC_NE2K;
+ } else if (STREQ(def->model, "pcnet")) {
+ *vendor = QEMU_PCI_VENDOR_AMD;
+ *product = QEMU_PCI_PRODUCT_NIC_PCNET;
+ } else if (STREQ(def->model, "rtl8139")) {
+ *vendor = QEMU_PCI_VENDOR_REALTEK;
+ *product = QEMU_PCI_PRODUCT_NIC_RTL8139;
+ } else if (STREQ(def->model, "e1000")) {
+ *vendor = QEMU_PCI_VENDOR_INTEL;
+ *product = QEMU_PCI_PRODUCT_NIC_E1000;
+ } else if (STREQ(def->model, "virtio")) {
+ *vendor = QEMU_PCI_VENDOR_REDHAT;
+ *product = QEMU_PCI_PRODUCT_NIC_VIRTIO;
+ } else {
+ VIR_INFO("Unexpected NIC model %s, cannot get PCI address",
+ def->model);
+ return -1;
+ }
+ return 0;
+}
+
+static int
+qemuGetPCIControllerVendorProduct(virDomainControllerDefPtr def,
+ unsigned *vendor,
+ unsigned *product)
+{
+ switch (def->type) {
+ case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
+ *vendor = QEMU_PCI_VENDOR_LSI_LOGIC;
+ *product = QEMU_PCI_PRODUCT_CONTROLLER_LSI;
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
+ /* XXX we could put in the ISA bridge address, but
+ that's not technically the FDC's address */
+ return -1;
+
+ case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
+ *vendor = QEMU_PCI_VENDOR_INTEL;
+ *product = QEMU_PCI_PRODUCT_CONTROLLER_PIIX;
+ break;
+
+ default:
+ VIR_INFO("Unexpected controller type %s, cannot get PCI address",
+ virDomainControllerTypeToString(def->type));
+ return -1;
+ }
+
+ return 0;
+}
+
+static int
+qemuGetPCIVideoVendorProduct(virDomainVideoDefPtr def,
+ unsigned *vendor,
+ unsigned *product)
+{
+ switch (def->type) {
+ case VIR_DOMAIN_VIDEO_TYPE_CIRRUS:
+ *vendor = QEMU_PCI_VENDOR_CIRRUS;
+ *product = QEMU_PCI_PRODUCT_VGA_CIRRUS;
+ break;
+
+ case VIR_DOMAIN_VIDEO_TYPE_VGA:
+ *vendor = QEMU_PCI_VENDOR_QEMU;
+ *product = QEMU_PCI_PRODUCT_VGA_STDVGA;
+ break;
+
+ case VIR_DOMAIN_VIDEO_TYPE_VMVGA:
+ *vendor = QEMU_PCI_VENDOR_VMWARE;
+ *product = QEMU_PCI_PRODUCT_VGA_VMWARE;
+ break;
+
+ default:
+ return -1;
+ }
+ return 0;
+}
+
+static int
+qemuGetPCISoundVendorProduct(virDomainSoundDefPtr def,
+ unsigned *vendor,
+ unsigned *product)
+{
+ switch (def->model) {
+ case VIR_DOMAIN_SOUND_MODEL_ES1370:
+ *vendor = QEMU_PCI_VENDOR_ENSONIQ;
+ *product = QEMU_PCI_PRODUCT_AUDIO_ES1370;
+ break;
+
+ case VIR_DOMAIN_SOUND_MODEL_AC97:
+ *vendor = QEMU_PCI_VENDOR_INTEL;
+ *product = QEMU_PCI_PRODUCT_AUDIO_AC97;
+ break;
+
+ default:
+ return -1;
+ }
+
+ return 0;
+}
+
+static int
+qemuGetPCIWatchdogVendorProduct(virDomainWatchdogDefPtr def,
+ unsigned *vendor,
+ unsigned *product)
+{
+ switch (def->model) {
+ case VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB:
+ *vendor = QEMU_PCI_VENDOR_INTEL;
+ *product = QEMU_PCI_PRODUCT_WATCHDOG_I63000ESB;
+ break;
+
+ default:
+ return -1;
+ }
+
+ return 0;
+}
+
+
+/*
+ * This entire method assumes that PCI devices in 'info pci'
+ * match ordering of devices specified on the command line
+ * wrt to devices of matching vendor+product
+ *
+ * XXXX this might not be a valid assumption if we assign
+ * some static addrs on CLI. Have to check that...
+ */
+static int
+qemuAssignPCIAddresses(virDomainObjPtr vm,
+ qemuMonitorPCIAddress *addrs,
+ int naddrs)
+{
+ unsigned int vendor = 0, product = 0;
+ int i;
+
+ /* XXX should all these vendor/product IDs be kept in the
+ * actual device data structure instead ?
+ */
+
+ for (i = 0 ; i < vm->def->ndisks ; i++) {
+ if (qemuGetPCIDiskVendorProduct(vm->def->disks[i], &vendor,
&product) < 0)
+ continue;
+
+ if (qemuAssignNextPCIAddress(&(vm->def->disks[i]->addr),
+ vendor, product,
+ addrs, naddrs) < 0) {
+ qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("cannot find PCI address for VirtIO disk %s"),
+ vm->def->disks[i]->dst);
+ return -1;
+ }
+ }
+
+ for (i = 0 ; i < vm->def->nnets ; i++) {
+ if (qemuGetPCINetVendorProduct(vm->def->nets[i], &vendor, &product)
< 0)
+ continue;
+
+ if (qemuAssignNextPCIAddress(&(vm->def->nets[i]->addr),
+ vendor, product,
+ addrs, naddrs) < 0) {
+ qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("cannot find PCI address for %s NIC"),
+ vm->def->nets[i]->model);
+ return -1;
+ }
+ }
+
+ for (i = 0 ; i < vm->def->ncontrollers ; i++) {
+ if (qemuGetPCIControllerVendorProduct(vm->def->controllers[i], &vendor,
&product) < 0)
+ continue;
+
+ if (qemuAssignNextPCIAddress(&(vm->def->controllers[i]->addr),
+ vendor, product,
+ addrs, naddrs) < 0) {
+ qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("cannot find PCI address for controller %s"),
+
virDomainControllerTypeToString(vm->def->controllers[i]->type));
+ return -1;
+ }
+ }
+
+ for (i = 0 ; i < vm->def->nvideos ; i++) {
+ if (qemuGetPCIVideoVendorProduct(vm->def->videos[i], &vendor,
&product) < 0)
+ continue;
+
+ if (qemuAssignNextPCIAddress(&(vm->def->videos[i]->addr),
+ vendor, product,
+ addrs, naddrs) < 0) {
+ qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("cannot find PCI address for video adapter
%s"),
+
virDomainVideoTypeToString(vm->def->videos[i]->type));
+ return -1;
+ }
+ }
+
+ for (i = 0 ; i < vm->def->nsounds ; i++) {
+ if (qemuGetPCISoundVendorProduct(vm->def->sounds[i], &vendor,
&product) < 0)
+ continue;
+
+ if (qemuAssignNextPCIAddress(&(vm->def->sounds[i]->addr),
+ vendor, product,
+ addrs, naddrs) < 0) {
+ qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("cannot find PCI address for sound adapter
%s"),
+
virDomainSoundModelTypeToString(vm->def->sounds[i]->model));
+ return -1;
+ }
+ }
+
+
+ if (qemuGetPCIWatchdogVendorProduct(vm->def->watchdog, &vendor,
&product) == 0) {
+ if (qemuAssignNextPCIAddress(&(vm->def->watchdog->addr),
+ vendor, product,
+ addrs, naddrs) < 0) {
+ qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("cannot find PCI address for watchdog %s"),
+
virDomainWatchdogModelTypeToString(vm->def->watchdog->model));
+ return -1;
+ }
+ }
+
+ /* XXX console (virtio) */
+
+
+ /* ... and now things we don't have in our xml */
+
+ /* XXX USB controller ? */
+
+ /* XXXX virtio balloon ? */
+
+ /* XXX what about other PCI devices (ie bridges) */
+
+ return 0;
+}
+
+static int
+qemuInitPCIAddresses(struct qemud_driver *driver,
+ virDomainObjPtr vm)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ int naddrs;
+ int ret;
+ qemuMonitorPCIAddress *addrs = NULL;
+
+ qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ naddrs = qemuMonitorGetAllPCIAddresses(priv->mon,
+ &addrs);
+ qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+ ret = qemuAssignPCIAddresses(vm, addrs, naddrs);
+
+ VIR_FREE(addrs);
+
+ return ret;
+}
+
static int qemudNextFreeVNCPort(struct qemud_driver *driver ATTRIBUTE_UNUSED) {
int i;
@@ -2471,6 +2833,9 @@ static int qemudStartVMDaemon(virConnectPtr conn,
if (qemuInitPasswords(driver, vm) < 0)
goto abort;
+ if (qemuInitPCIAddresses(driver, vm) < 0)
+ goto abort;
+
qemuDomainObjEnterMonitorWithDriver(driver, vm);
if (qemuMonitorSetBalloon(priv->mon, vm->def->memory) < 0) {
qemuDomainObjExitMonitorWithDriver(driver, vm);
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 30f2cce..e41ae58 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1276,3 +1276,16 @@ int qemuMonitorAttachDrive(qemuMonitorPtr mon,
return ret;
}
+
+int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon,
+ qemuMonitorPCIAddress **addrs)
+{
+ DEBUG("mon=%p, fd=%d addrs=%p", mon, mon->fd, addrs);
+ int ret;
+
+ if (mon->json)
+ ret = qemuMonitorJSONGetAllPCIAddresses(mon, addrs);
+ else
+ ret = qemuMonitorTextGetAllPCIAddresses(mon, addrs);
+ return ret;
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index fcfdcd5..c15942b 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -269,4 +269,15 @@ int qemuMonitorAttachDrive(qemuMonitorPtr mon,
virDomainDevicePCIAddress *controllerAddr,
virDomainDeviceDriveAddress *driveAddr);
+
+typedef struct _qemuMonitorPCIAddress qemuMonitorPCIAddress;
+struct _qemuMonitorPCIAddress {
+ unsigned int vendor;
+ unsigned int product;
+ virDomainDevicePCIAddress addr;
+};
+
+int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon,
+ qemuMonitorPCIAddress **addrs);
+
#endif /* QEMU_MONITOR_H */
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 61df373..3dbdb9b 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1588,3 +1588,10 @@ int qemuMonitorJSONAttachDrive(qemuMonitorPtr mon,
virJSONValueFree(reply);
return ret;
}
+
+
+int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
+ qemuMonitorPCIAddress **addrs ATTRIBUTE_UNUSED)
+{
+ return -1;
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 1997dbb..f7fd19f 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -149,4 +149,7 @@ int qemuMonitorJSONAttachDrive(qemuMonitorPtr mon,
virDomainDevicePCIAddress *controllerAddr,
virDomainDeviceDriveAddress *driveAddr);
+int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon,
+ qemuMonitorPCIAddress **addrs);
+
#endif /* QEMU_MONITOR_JSON_H */
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 16bc81f..db0cb76 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -1722,3 +1722,132 @@ cleanup:
}
+/*
+ * The format we're after looks like this
+ *
+ * (qemu) info pci
+ * Bus 0, device 0, function 0:
+ * Host bridge: PCI device 8086:1237
+ * id ""
+ * Bus 0, device 1, function 0:
+ * ISA bridge: PCI device 8086:7000
+ * id ""
+ * Bus 0, device 1, function 1:
+ * IDE controller: PCI device 8086:7010
+ * BAR4: I/O at 0xc000 [0xc00f].
+ * id ""
+ * Bus 0, device 1, function 3:
+ * Bridge: PCI device 8086:7113
+ * IRQ 9.
+ * id ""
+ * Bus 0, device 2, function 0:
+ * VGA controller: PCI device 1013:00b8
+ * BAR0: 32 bit prefetchable memory at 0xf0000000 [0xf1ffffff].
+ * BAR1: 32 bit memory at 0xf2000000 [0xf2000fff].
+ * id ""
+ * Bus 0, device 3, function 0:
+ * Ethernet controller: PCI device 8086:100e
+ * IRQ 11.
+ * BAR0: 32 bit memory at 0xf2020000 [0xf203ffff].
+ * BAR1: I/O at 0xc040 [0xc07f].
+ * id ""
+ *
+ * Of this, we're interesting in the vendor/product ID
+ * and the bus/device/function data.
+ */
+#define CHECK_END(p) if (!(p)) break;
+#define SKIP_TO(p, lbl) \
+ (p) = strstr((p), (lbl)); \
+ if (p) \
+ (p) += strlen(lbl);
+#define GET_INT(p, base, val) \
+ if (virStrToLong_ui((p), &(p), (base), &(val)) < 0) { \
+ qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED, \
+ _("cannot parse value for %s"), #val); \
+ break; \
+ }
+#define SKIP_SPACE(p) \
+ while (*(p) == ' ') (p)++;
+
+int qemuMonitorTextGetAllPCIAddresses(qemuMonitorPtr mon,
+ qemuMonitorPCIAddress **retaddrs)
+{
+ char *reply;
+ qemuMonitorPCIAddress *addrs = NULL;
+ int naddrs = 0;
+ char *p;
+
+ *retaddrs = NULL;
+
+ if (qemuMonitorCommand(mon, "info pci", &reply) < 0) {
+ qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+ _("cannot query PCI addresses"));
+ return -1;
+ }
+
+ p = reply;
+
+
+ while (p) {
+ unsigned int bus, slot, func, vendor, product;
+
+ SKIP_TO(p, " Bus");
+ CHECK_END(p);
+ SKIP_SPACE(p);
+ GET_INT(p, 10, bus);
+ CHECK_END(p);
+
+ SKIP_TO(p, ", device");
+ CHECK_END(p);
+ SKIP_SPACE(p);
+ GET_INT(p, 10, slot);
+ CHECK_END(p);
+
+ SKIP_TO(p, ", function");
+ CHECK_END(p);
+ SKIP_SPACE(p);
+ GET_INT(p, 10, func);
+ CHECK_END(p);
+
+ SKIP_TO(p, "PCI device");
+ CHECK_END(p);
+ SKIP_SPACE(p);
+ GET_INT(p, 16, vendor);
+ CHECK_END(p);
+
+ if (*p != ':')
+ break;
+ p++;
+ GET_INT(p, 16, product);
+
+ if (VIR_REALLOC_N(addrs, naddrs+1) < 0) {
+ virReportOOMError(NULL);
+ goto error;
+ }
+
+ addrs[naddrs].addr.domain = 0;
+ addrs[naddrs].addr.bus = bus;
+ addrs[naddrs].addr.slot = slot;
+ addrs[naddrs].addr.function = func;
+ addrs[naddrs].vendor = vendor;
+ addrs[naddrs].product = product;
+ naddrs++;
+
+ VIR_DEBUG("Got dev %d:%d:%d %x:%x", bus, slot, func, vendor,
product);
+ }
+
+ VIR_FREE(reply);
+
+ *retaddrs = addrs;
+
+ return naddrs;
+
+error:
+ VIR_FREE(addrs);
+ VIR_FREE(reply);
+ return -1;
+}
+#undef GET_INT
+#undef SKIP_SPACE
+#undef CHECK_END
+#undef SKIP_TO
diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h
index 94fcc02..e39a4a5 100644
--- a/src/qemu/qemu_monitor_text.h
+++ b/src/qemu/qemu_monitor_text.h
@@ -149,5 +149,7 @@ int qemuMonitorTextAttachDrive(qemuMonitorPtr mon,
virDomainDevicePCIAddress *controllerAddr,
virDomainDeviceDriveAddress *driveAddr);
+int qemuMonitorTextGetAllPCIAddresses(qemuMonitorPtr mon,
+ qemuMonitorPCIAddress **addrs);
#endif /* QEMU_MONITOR_TEXT_H */
--
1.6.5.2
5:44 a.m.
New subject: [libvirt] [PATCH 12/12] Detect PCI addresses at QEMU startup
Hey,
This looks good, except we're encoding a lot more knowledge of how qemu
works than I'd like.
The plan we had to future proof this was that libvirt would assign an id
(using -device foo,id=bar) and 'info pci' would include the device id in
its output.
Once you have that, you no longer need to make assumptions about what
product:vendor is used for each device or what order addresses are
allocated. It's a simple addition to qemu and a simple addition to your
'info pci' parsing code, but I think we'd rest easier wrt. future
compatibility.
Random thought - how well does this patch work if e.g. you don't specify
a NIC model? I don't see where we hard-code the knowledge about what
model qemu uses by default ...
Also, it'd be good to have unit tests for this - inputs would be a
domain XML lacking addresses, the corresponding 'info pci' output and it
would check addresses against an expected resulting domain XML.
Cheers,
Mark.
7:59 a.m.
New subject: [libvirt] [PATCH 12/12] Detect PCI addresses at QEMU startup
On Fri, Dec 18, 2009 at 11:44:54AM +0000, Mark McLoughlin wrote:
Hey,
This looks good, except we're encoding a lot more knowledge of how qemu
works than I'd like.
The plan we had to future proof this was that libvirt would assign an id
(using -device foo,id=bar) and 'info pci' would include the device id in
its output.
Once you have that, you no longer need to make assumptions about what
product:vendor is used for each device or what order addresses are
allocated. It's a simple addition to qemu and a simple addition to your
'info pci' parsing code, but I think we'd rest easier wrt. future
compatibility.
That would be preferable, but the trouble is that will only work with
QEMU >= 0.12. Using the 'info pci' output works for all QEMUs. By
'works'
I mean it lets us handle unplug even of devices present at startup. It
obviously doesn't allow for static PCI addresing, since that requires
the new -device arg.
Random thought - how well does this patch work if e.g. you don't
specify
a NIC model? I don't see where we hard-code the knowledge about what
model qemu uses by default ...
The other patch series switching over to -device deals with that by
explicitly setting rtl8139 NIC if no model is provided, so we don't
rely on the unstable QEMU default anymore. Of course there is no
concept of defaults when using -device syntax.
Also, it'd be good to have unit tests for this - inputs would be
a
domain XML lacking addresses, the corresponding 'info pci' output and it
would check addresses against an expected resulting domain XML.
Agreed.
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 :|
8:04 a.m.
New subject: [libvirt] [PATCH 12/12] Detect PCI addresses at QEMU startup
On Fri, 2009-12-18 at 13:59 +0000, Daniel P. Berrange wrote:
> The plan we had to future proof this was that libvirt would
assign an id
> (using -device foo,id=bar) and 'info pci' would include the device id in
> its output.
>
> Once you have that, you no longer need to make assumptions about what
> product:vendor is used for each device or what order addresses are
> allocated. It's a simple addition to qemu and a simple addition to your
> 'info pci' parsing code, but I think we'd rest easier wrt. future
> compatibility.
That would be preferable, but the trouble is that will only work with
QEMU >= 0.12. Using the 'info pci' output works for all QEMUs. By
'works'
I mean it lets us handle unplug even of devices present at startup. It
obviously doesn't allow for static PCI addresing, since that requires
the new -device arg.
Yep, I didn't mean to suggest that you shouldn't go ahead with your
approach.
Just that if qemu had the id listed in 'info pci', libvirt could use
that with versions of qemu that supply it and we'd then be less exposed
to future changes.
Cheers,
Mark.
8:08 a.m.
New subject: [libvirt] [PATCH 12/12] Detect PCI addresses at QEMU startup
On Fri, Dec 18, 2009 at 02:04:04PM +0000, Mark McLoughlin wrote:
On Fri, 2009-12-18 at 13:59 +0000, Daniel P. Berrange wrote:
> > The plan we had to future proof this was that libvirt would assign an id
> > (using -device foo,id=bar) and 'info pci' would include the device id
in
> > its output.
> >
> > Once you have that, you no longer need to make assumptions about what
> > product:vendor is used for each device or what order addresses are
> > allocated. It's a simple addition to qemu and a simple addition to your
> > 'info pci' parsing code, but I think we'd rest easier wrt. future
> > compatibility.
>
> That would be preferable, but the trouble is that will only work with
> QEMU >= 0.12. Using the 'info pci' output works for all QEMUs. By
'works'
> I mean it lets us handle unplug even of devices present at startup. It
> obviously doesn't allow for static PCI addresing, since that requires
> the new -device arg.
Yep, I didn't mean to suggest that you shouldn't go ahead with your
approach.
Just that if qemu had the id listed in 'info pci', libvirt could use
that with versions of qemu that supply it and we'd then be less exposed
to future changes.
For QEMU >= 0.12, it might also be possible to use the 'info qtree' data
instead of 'info pci'
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 :|
5:59 a.m.
New subject: [libvirt] [PATCH 12/12] Detect PCI addresses at QEMU startup
On 12/18/09 15:04, Mark McLoughlin wrote:
Just that if qemu had the id listed in 'info pci', libvirt
could use
that with versions of qemu that supply it and we'd then be less exposed
to future changes.
qemu 0.12 has the id actually listed in 'info pci' ...
cheers,
Gerd
6:06 a.m.
New subject: [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
9:40 a.m.
New subject: [libvirt] Re: [PATCH 0/12] Standardized device addressing & SCSI controller/disk hotplug
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.
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 :|
9:53 a.m.
New subject: [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
10:07 a.m.
New subject: [libvirt] Re: [PATCH 0/12] Standardized device addressing & SCSI controller/disk hotplug
On Tue, Dec 15, 2009 at 04:53:24PM +0100, Wolfgang Mauerer wrote:
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.
Ah, thanks for that. I'll include your patch anyway
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 :|
3:39 a.m.
On Thu, Dec 10, 2009 at 10:22:20PM +0000, Daniel P. Berrange wrote:
The XML for each address type looks like
<address type='pci' mode='static' domain='0x0000'
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.
The second thought is whether we should put the <address>
data inside a <device> element, so it looked like
<device type='pci'>
<address domain='0x0000' 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='0x0000' bus='0x1e' slot='0x07'
function='0x0'/>
</device>
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 :|
2:38 a.m.
Daniel P. Berrange wrote:
On Thu, Dec 10, 2009 at 10:22:20PM +0000, Daniel P. Berrange wrote:
> The XML for each address type looks like
>
> <address type='pci' mode='static' domain='0x0000'
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='0x0000' 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='0x0000' bus='0x1e' slot='0x07'
function='0x0'/>
</device>
Can't certainly do any harm to keep the format extensible.
Regards,
Wolfgang
5:17 a.m.
On Thu, 2009-12-17 at 09:39 +0000, Daniel P. Berrange wrote:
On Thu, Dec 10, 2009 at 10:22:20PM +0000, Daniel P. Berrange wrote:
> The XML for each address type looks like
>
> <address type='pci' mode='static' domain='0x0000'
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.
Just reading the proposal now and this is the bit that jumps out at me
most. I don't think we need the dynamic/static mode split.
For PCI, we really want to just automatically remember assigned PCI
addresses and use them forever after - I don't see any obvious use case
where you wouldn't want this.
Also, it strikes me that we've done this before - e.g. if you define a
guest with an interface with no explicit MAC address, we generate one
and save it in the config. Which is pretty much the same thing from the
users point of view.
The second thought is whether we should put the <address>
data inside a <device> element, so it looked like
<device type='pci'>
<address domain='0x0000' 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='0x0000' bus='0x1e' slot='0x07'
function='0x0'/>
</device>
Dunno, it seems odd to have:
<devices>
<interface>
<device>
I read that as "<devices> is a list of device objects",
"<interface> is
a specialization of the device class" ... "wtf is <device> then?"
I guess it makes some sense as a container of device information common
to all device types, though.
Cheers,
Mark.
8:07 a.m.
On Fri, Dec 18, 2009 at 11:17:59AM +0000, Mark McLoughlin wrote:
On Thu, 2009-12-17 at 09:39 +0000, Daniel P. Berrange wrote:
> On Thu, Dec 10, 2009 at 10:22:20PM +0000, Daniel P. Berrange wrote:
> > The XML for each address type looks like
> >
> > <address type='pci' mode='static'
domain='0x0000' 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.
Just reading the proposal now and this is the bit that jumps out at me
most. I don't think we need the dynamic/static mode split.
For PCI, we really want to just automatically remember assigned PCI
addresses and use them forever after - I don't see any obvious use case
where you wouldn't want this.
Also, it strikes me that we've done this before - e.g. if you define a
guest with an interface with no explicit MAC address, we generate one
and save it in the config. Which is pretty much the same thing from the
users point of view.
The thing I'm trying to avoid is having libvirt update the persistent
config files outside the context of the existing virDefineXML API, and
updating the live state file only helps for the duration the that VM
is running. Hence I came up with the static vs dynamic idea, where the
app would see the dynamic address, dump the XML and re-define it with
static addresses.
The only other option I see is to explicitly assign all PCI addresses
at time of virDefineXML, and then always pass an address on the command
line to QEMU for all devices, but this doesn't work for older QEMU where
we can't set PCI addresses.
> The second thought is whether we should put the <address>
> data inside a <device> element, so it looked like
>
> <device type='pci'>
> <address domain='0x0000' 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='0x0000' bus='0x1e' slot='0x07'
function='0x0'/>
> </device>
Dunno, it seems odd to have:
<devices>
<interface>
<device>
I read that as "<devices> is a list of device objects",
"<interface> is
a specialization of the device class" ... "wtf is <device> then?"
I guess it makes some sense as a container of device information common
to all device types, though.
The idea is that <device> gives information about the physical hardware
being emulated, and its contents are interpreted relative to the type
attribute. If we were defining libvirt XML again from scratch, it would
have been preferrable to have the relationship inverted really.
<devices>
<device>
<interface>
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 :|
8:18 a.m.
On Fri, 2009-12-18 at 14:07 +0000, Daniel P. Berrange wrote:
On Fri, Dec 18, 2009 at 11:17:59AM +0000, Mark McLoughlin wrote:
> On Thu, 2009-12-17 at 09:39 +0000, Daniel P. Berrange wrote:
> > On Thu, Dec 10, 2009 at 10:22:20PM +0000, Daniel P. Berrange wrote:
> > > The XML for each address type looks like
> > >
> > > <address type='pci' mode='static'
domain='0x0000' 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.
>
> Just reading the proposal now and this is the bit that jumps out at me
> most. I don't think we need the dynamic/static mode split.
>
> For PCI, we really want to just automatically remember assigned PCI
> addresses and use them forever after - I don't see any obvious use case
> where you wouldn't want this.
>
> Also, it strikes me that we've done this before - e.g. if you define a
> guest with an interface with no explicit MAC address, we generate one
> and save it in the config. Which is pretty much the same thing from the
> users point of view.
The thing I'm trying to avoid is having libvirt update the persistent
config files outside the context of the existing virDefineXML API,
Remind me why are you trying to avoid that
(I've always been a bit dubious about virDefineXML saving a different
config from the one you've supplied, but that's a different matter)
and
updating the live state file only helps for the duration the that VM
is running. Hence I came up with the static vs dynamic idea, where the
app would see the dynamic address, dump the XML and re-define it with
static addresses.
The only other option I see is to explicitly assign all PCI addresses
at time of virDefineXML, and then always pass an address on the command
line to QEMU for all devices, but this doesn't work for older QEMU where
we can't set PCI addresses.
How about during virDefineXML, execing qemu, not starting the guest, but
instead just querying the device assignments and saving them? If we
ignored errors, it wouldn't change the semantics of the API.
Hmm. Even if you go with the 'dynamic' idea, you're kind of changing the
semantics anyway - you're saying the management apps should do 'define,
start, dumpxml, define' where they didn't have to do that before. And if
all apps should do that, we shouldn't 'start' do it automatically?
Cheers,
Mark.
8:32 a.m.
On Fri, Dec 18, 2009 at 02:18:32PM +0000, Mark McLoughlin wrote:
On Fri, 2009-12-18 at 14:07 +0000, Daniel P. Berrange wrote:
>
> The thing I'm trying to avoid is having libvirt update the persistent
> config files outside the context of the existing virDefineXML API,
Remind me why are you trying to avoid that
(I've always been a bit dubious about virDefineXML saving a different
config from the one you've supplied, but that's a different matter)
> and
> updating the live state file only helps for the duration the that VM
> is running. Hence I came up with the static vs dynamic idea, where the
> app would see the dynamic address, dump the XML and re-define it with
> static addresses.
>
> The only other option I see is to explicitly assign all PCI addresses
> at time of virDefineXML, and then always pass an address on the command
> line to QEMU for all devices, but this doesn't work for older QEMU where
> we can't set PCI addresses.
How about during virDefineXML, execing qemu, not starting the guest, but
instead just querying the device assignments and saving them? If we
ignored errors, it wouldn't change the semantics of the API.
Hmm. Even if you go with the 'dynamic' idea, you're kind of changing the
semantics anyway - you're saying the management apps should do 'define,
start, dumpxml, define' where they didn't have to do that before. And if
all apps should do that, we shouldn't 'start' do it automatically?
Actually I think I've been thinking about dynamic vs static at the wrong
level. It isn't something that needs to be considered per device. It is
a property of the QEMU version the guest runs, and we can figure that out
at time of define.
- For QEMU >= 0.12, all devices support static PCI addressing, so we
can easily assign all PCI addrs at time of virDefineXML and this
address info gets saved to disk.
- For QEMU < 0.12, no devices support static PCI addressing, so we will
always query for assigned PCI addrs at time of startup and have no
need to ever save this info to disk, since the QEMU doesn't support
static addressing
At most we need to indicate in the capabilities XML that a guest type
supports PCI addressing, merely as a piece of useful info for PCI. They
would never have to actually set any PCI addrs themselves.
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 :|
10:53 a.m.
On Fri, 2009-12-18 at 14:32 +0000, Daniel P. Berrange wrote:
Actually I think I've been thinking about dynamic vs static at
the wrong
level. It isn't something that needs to be considered per device. It is
a property of the QEMU version the guest runs, and we can figure that out
at time of define.
- For QEMU >= 0.12, all devices support static PCI addressing, so we
can easily assign all PCI addrs at time of virDefineXML and this
address info gets saved to disk.
How would you do the address allocation in libvirt? Would you start qemu
and query which addresses have been assigned already? Would you have
knowledge in libvirt about e.g. how many devices can be assigned to a
bus?
- For QEMU < 0.12, no devices support static PCI addressing, so
we will
always query for assigned PCI addrs at time of startup and have no
need to ever save this info to disk, since the QEMU doesn't support
static addressing
It'd be nice to remember the assigned addresses for a 0.11 guest so that
when the host is upgraded to 0.12 we can request the same addresses that
were used when running on 0.11.
At most we need to indicate in the capabilities XML that a guest
type
supports PCI addressing, merely as a piece of useful info for PCI. They
would never have to actually set any PCI addrs themselves.
You've kind of lost me here. From the API user's point of view, there
are two ways you might want this handled:
1) Don't specify a device address in the guest config, but whatever
address ultimately gets assigned is then stored in the guest
config so that it never changes
I don't see that it matters so much when the address assignment
happens - e.g. it could be assigned at 'define' or assigned at
'start' - all that matters is once the guest OS has seen the
device assigned at that address, then the address shouldn't change
thereafter
2) Obtain information on what addresses are available for assignment
and assign an address to a device before attaching it to the guest
config
Personally, I don't think this is terribly useful and I'm not sure
we're totally clear on the real details of the use case - e.g. we
could have a "give me an unallocated address of type 'PCI'" API,
but why would someone use that instead of (1). We could have a
"give me a list of of available slots for bus Y" API, but if we
were e.g. going to have a PCI specific dialog in virt-manager for
arranging the PCI topology, maybe it's fine for virt-manager to
have a load of PCI specific knowledge.
So, I think we're really just talking about (1), but it's a question of
whether libvirt should allow qemu to allocate addresses, or have libvirt
itself allocate them.
I think it's easier to just allow qemu allocate the addresses and have
libvirt query the result. But this means changing the guest config at
'start', which you're saying we should avoid. (Why?)
If libvirt does the address assignment itself at 'define', I guess we
only have two fairly minor problems - a) making sure the allocation
scheme is safe against future qemu changes and b) not being able to have
address for guests created on a host with < 0.12.
Cheers,
Mark.
11:02 a.m.
On Mon, Dec 21, 2009 at 04:53:10PM +0000, Mark McLoughlin wrote:
On Fri, 2009-12-18 at 14:32 +0000, Daniel P. Berrange wrote:
> Actually I think I've been thinking about dynamic vs static at the wrong
> level. It isn't something that needs to be considered per device. It is
> a property of the QEMU version the guest runs, and we can figure that out
> at time of define.
>
> - For QEMU >= 0.12, all devices support static PCI addressing, so we
> can easily assign all PCI addrs at time of virDefineXML and this
> address info gets saved to disk.
How would you do the address allocation in libvirt? Would you start qemu
and query which addresses have been assigned already? Would you have
knowledge in libvirt about e.g. how many devices can be assigned to a
bus?
Either start QEMU & query it, or manually assign the addresses. Since
we are using the -nodefaults argument, we know there are no default
devices in QEMU with the exception of a PIIX3. So we can easily just
assign addresses starting at slot 2. We don't need to know how many
addresses are possible on a bus, because if we assign a slot number
that doesn't exist, then QEMU will now print an error at startup instead
of core dumping like it did previously.
> - For QEMU < 0.12, no devices support static PCI addressing,
so we will
> always query for assigned PCI addrs at time of startup and have no
> need to ever save this info to disk, since the QEMU doesn't support
> static addressing
It'd be nice to remember the assigned addresses for a 0.11 guest so that
when the host is upgraded to 0.12 we can request the same addresses that
were used when running on 0.11.
I don't think thats worth bothering about since anyone using 0.11 is
having to cope with dynamic addressing.
> At most we need to indicate in the capabilities XML that a guest
type
> supports PCI addressing, merely as a piece of useful info for PCI. They
> would never have to actually set any PCI addrs themselves.
You've kind of lost me here. From the API user's point of view, there
are two ways you might want this handled:
1) Don't specify a device address in the guest config, but whatever
address ultimately gets assigned is then stored in the guest
config so that it never changes
I don't see that it matters so much when the address assignment
happens - e.g. it could be assigned at 'define' or assigned at
'start' - all that matters is once the guest OS has seen the
device assigned at that address, then the address shouldn't change
thereafter
2) Obtain information on what addresses are available for assignment
and assign an address to a device before attaching it to the guest
config
Personally, I don't think this is terribly useful and I'm not sure
we're totally clear on the real details of the use case - e.g. we
could have a "give me an unallocated address of type 'PCI'" API,
but why would someone use that instead of (1). We could have a
"give me a list of of available slots for bus Y" API, but if we
were e.g. going to have a PCI specific dialog in virt-manager for
arranging the PCI topology, maybe it's fine for virt-manager to
have a load of PCI specific knowledge.
So, I think we're really just talking about (1), but it's a question of
whether libvirt should allow qemu to allocate addresses, or have libvirt
itself allocate them.
The capabilities flag is just needed so that an app can validate that
libvirt does indeed support static addressing. It allows it to change
behaviour should it deem it neccessary. eg iif static addressing is
not supported, the mgmt app might like to avoid PCI hotplug, and only
allow plugging in SCSI disks to an existing controller.
I think it's easier to just allow qemu allocate the addresses and
have
libvirt query the result. But this means changing the guest config at
'start', which you're saying we should avoid. (Why?)
Simply because starting a VM should not modify its config, since that
violates the declared semantics of the start API.
If libvirt does the address assignment itself at 'define', I
guess we
only have two fairly minor problems - a) making sure the allocation
scheme is safe against future qemu changes and b) not being able to have
address for guests created on a host with < 0.12.
I don't see a) being a problem anymore, since the -nodefaults argument
allows us to turn off everything QEMU does by default, so it only creates
devices for things we explicitly ask for.
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 :|
5:24 a.m.
Hi Dan,
Looks great overall
On Thu, 2009-12-10 at 22:22 +0000, Daniel P. Berrange wrote:
I then remembered that for support NIC/VirtIO/hostdev disk unplug
Mark M had previously added PCI address information to the internal
XML state files for <interface>, <disk> and <hostdev> elements.
Note, the devaddr property in the state file only had domain/bus/slot
since there was no multi-function devices involved.
Also, we should try to handle translating the devaddr state to <address>
config - e.g. if you upgrade libvirt while a guest is running with a
hotplugged device
libvirt itself will auto-assign all drive addresses, and QEMU
will auto-assign PCI adresses in dynamic mode. When starting
a guest VM we run 'info pci' to get a list of PCI vendor/product
IDs and matching PCI addresses. We then attempt to match those
up with the devices we specified to QEMU. It sounds nasty, but
it actually works fairly well. This means we also now make it
possible to hotunplug any device, even those the VM was initially
booted with
Glad to hear this works well
There is one small issue with all this, we need to know every
PCI device in the guest. It turns out there are a handful of
devices in QEMU we don't represent in XML yet
- Virtio balloon device
- Virtio console (this is easy to address with matt's patches)
- ISA bridge
- USB controller
- Some kind of PCI bridge (not clear what this is, it has
PCI ID 8086:7113
If an management application is to be able to fully control
static PCI addressing, we need to represent these somehow,
so apps can give them addresses. Technically we could get
away with not representing the ISA/PCI bridge since QEMU
always gives them the first PCI slot no matter what. Still
need the VirtIO & USB devices dealt with.
When we discussed this on qemu-devel, we thought it made sense to only
expose "free slots" information to the higher levels - i.e. all the
management app cares about is what slots are available for address
allocation, not what is in the used slots. It also means the management
apps doesn't need to know e.g. how many PCI slots there are per bus.
Cheers,
Mark.
5435
days inactive
5460
days old
34 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Gerd Hoffmann -
Mark McLoughlin -
Wolfgang Mauerer