From: Daniel P. Berrange <berrange@redhat.com>
Date: Mon, 30 Nov 2009 18:35:58 +0000 (+0000)
Subject: Introduce a standardized data structure for device addresses
X-Git-Tag: v0.7.6~184
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1b0cce7d3a458839367f4a12c1300ebe1cdf5972;p=thirdparty%2Flibvirt.git

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,
  };

  struct _virDomainDevicePCIAddress {
    unsigned int domain;
    unsigned int bus;
    unsigned int slot;
    unsigned int function;
  };

  struct _virDomainDeviceInfo {
    int type;
    union {
        virDomainDevicePCIAddress pci;
    } addr;
  };

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' domain='0x0000' bus='0x1e' slot='0x07' function='0x0'/>

The structure definition is based on Wolfgang Mauerer's disk
controller patch series.

* docs/schemas/domain.rng: Define the <address> syntax and
  associate it with disk/net/hostdev devices
* src/conf/domain_conf.h, src/conf/domain_conf.c,
  src/libvirt_private.syms: APIs for parsing/formatting address
  information. Also remove the QEMU specific 'pci_addr' attributes
* src/qemu/qemu_driver.c: Replace use of 'pci_addr' attrs with
  new standardized format.
---

diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index 566b11753c..7e00e7fe9c 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -385,6 +385,9 @@
     <optional>
       <ref name="encryption"/>
     </optional>
+    <optional>
+      <ref name="address"/>
+    </optional>
   </define>
   <!--
       A disk description can be either of type file or block
@@ -715,6 +718,9 @@
           <empty/>
         </element>
       </optional>
+      <optional>
+	<ref name="address"/>
+      </optional>
     </interleave>
   </define>
   <!--
@@ -1134,10 +1140,15 @@
           <choice>
             <ref name="usbproduct"/>
             <ref name="usbaddress"/>
-            <ref name="pciaddress"/>
+            <element name="address">
+              <ref name="pciaddress"/>
+            </element>
           </choice>
         </element>
       </group>
+      <optional>
+        <ref name="address"/>
+      </optional>
     </element>
   </define>
   <define name="usbproduct">
@@ -1163,22 +1174,20 @@
     </element>
   </define>
   <define name="pciaddress">
-    <element name="address">
-      <optional>
-        <attribute name="domain">
-          <ref name="pciDomain"/>
-        </attribute>
-      </optional>
-      <attribute name="bus">
-        <ref name="pciBus"/>
-      </attribute>
-      <attribute name="slot">
-        <ref name="pciSlot"/>
-      </attribute>
-      <attribute name="function">
-        <ref name="pciFunc"/>
+    <optional>
+      <attribute name="domain">
+        <ref name="pciDomain"/>
       </attribute>
-    </element>
+    </optional>
+    <attribute name="bus">
+      <ref name="pciBus"/>
+    </attribute>
+    <attribute name="slot">
+      <ref name="pciSlot"/>
+    </attribute>
+    <attribute name="function">
+      <ref name="pciFunc"/>
+    </attribute>
   </define>
   <!--
       Devices attached to a domain.
@@ -1286,6 +1295,20 @@
       </interleave>
     </element>
   </define>
+
+  <define name="address">
+    <element name="address">
+      <choice>
+	<group>
+	  <attribute name="type">
+	    <value>pci</value>
+	  </attribute>
+	  <ref name="pciaddress"/>
+	</group>
+      </choice>
+    </element>
+  </define>
+
   <!--
        Type library
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0b4fe8bfd1..f199c08511 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -88,6 +88,10 @@ 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(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST,
               "block",
               "file",
@@ -357,6 +361,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
     VIR_FREE(def->driverName);
     VIR_FREE(def->driverType);
     virStorageEncryptionFree(def->encryption);
+    virDomainDeviceInfoClear(&def->info);
 
     VIR_FREE(def);
 }
@@ -408,8 +413,10 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
     }
 
     VIR_FREE(def->ifname);
-    VIR_FREE(def->nic_name);
     VIR_FREE(def->hostnet_name);
+
+    virDomainDeviceInfoClear(&def->info);
+
     VIR_FREE(def);
 }
 
@@ -483,6 +490,7 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def)
         return;
 
     VIR_FREE(def->target);
+    virDomainDeviceInfoClear(&def->info);
     VIR_FREE(def);
 }
 
@@ -731,6 +739,220 @@ void virDomainRemoveInactive(virDomainObjListPtr doms,
 }
 
 
+int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info,
+                                  int type)
+{
+    if (info->type != type)
+        return 0;
+
+    switch (info->type) {
+    case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
+        return virDomainDevicePCIAddressIsValid(&info->addr.pci);
+    }
+
+    return 0;
+}
+
+
+int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr)
+{
+    return addr->domain || addr->bus || addr->slot;
+}
+
+
+int virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info)
+{
+    if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+        return 1;
+    return 0;
+}
+
+
+void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
+{
+    memset(&info->addr, 0, sizeof(info->addr));
+    info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
+}
+
+
+/* Generate a string representation of a device address
+ * @param address Device address to stringify
+ */
+static int virDomainDeviceInfoFormat(virBufferPtr buf,
+                                     virDomainDeviceInfoPtr info)
+{
+    if (!info) {
+        virDomainReportError(NULL, VIR_ERR_INTERNAL_ERROR, "%s",
+                             _("missing device information"));
+        return -1;
+    }
+
+    if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+        return 0;
+
+    /* We'll be in domain/devices/[device type]/ so 3 level indent */
+    virBufferVSprintf(buf, "      <address type='%s'",
+                      virDomainDeviceAddressTypeToString(info->type));
+
+    switch (info->type) {
+    case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
+        virBufferVSprintf(buf, " domain='0x%.4x' bus='0x%.2x' slot='0x%.2x' function='0x%.1x'",
+                          info->addr.pci.domain,
+                          info->addr.pci.bus,
+                          info->addr.pci.slot,
+                          info->addr.pci.function);
+        break;
+
+    default:
+        virDomainReportError(NULL, VIR_ERR_INTERNAL_ERROR,
+                             _("unknown address type '%d'"), info->type);
+        return -1;
+    }
+
+    virBufferAddLit(buf, "/>\n");
+
+    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
+virDomainDeviceInfoParseXML(virConnectPtr conn,
+                            xmlNodePtr node,
+                            virDomainDeviceInfoPtr info,
+                            unsigned int flags ATTRIBUTE_UNUSED)
+{
+    xmlNodePtr cur;
+    xmlNodePtr address = NULL;
+    char *type = NULL;
+    int ret = -1;
+
+    virDomainDeviceInfoClear(info);
+
+    cur = node->children;
+    while (cur != NULL) {
+        if (cur->type == XML_ELEMENT_NODE) {
+            if (address == NULL &&
+                xmlStrEqual(cur->name, BAD_CAST "address")) {
+                address = cur;
+            }
+        }
+        cur = cur->next;
+    }
+
+    if (!address)
+        return 0;
+
+    type = virXMLPropString(address, "type");
+
+    if (type) {
+        if ((info->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;
+    }
+
+    switch (info->type) {
+    case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
+        if (virDomainDevicePCIAddressParseXML(conn, address, &info->addr.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);
+    return ret;
+}
+
+
 /* Parse the XML definition for a disk
  * @param node XML nodeset to parse for disk definition
  */
@@ -819,6 +1041,7 @@ virDomainDiskDefParseXML(virConnectPtr conn,
                 def->shared = 1;
             } else if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) &&
                        xmlStrEqual(cur->name, BAD_CAST "state")) {
+                /* Legacy back-compat. Don't add any more attributes here */
                 devaddr = virXMLPropString(cur, "devaddr");
             } else if (encryption == NULL &&
                        xmlStrEqual(cur->name, BAD_CAST "encryption")) {
@@ -928,15 +1151,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->info.addr.pci.domain,
+                   &def->info.addr.pci.bus,
+                   &def->info.addr.pci.slot) < 3) {
+            virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                                 _("Unable to parse devaddr parameter '%s'"),
+                                 devaddr);
+            goto error;
+        }
+        def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+    } else {
+        if (virDomainDeviceInfoParseXML(conn, node, &def->info, flags) < 0)
+            goto error;
     }
 
     def->src = source;
@@ -1153,6 +1381,7 @@ virDomainNetDefParseXML(virConnectPtr conn,
                 model = virXMLPropString(cur, "type");
             } else if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) &&
                        xmlStrEqual(cur->name, BAD_CAST "state")) {
+                /* Legacy back-compat. Don't add any more attributes here */
                 nic_name = virXMLPropString(cur, "nic");
                 hostnet_name = virXMLPropString(cur, "hostnet");
                 devaddr = virXMLPropString(cur, "devaddr");
@@ -1173,14 +1402,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->info.addr.pci.domain,
+                   &def->info.addr.pci.bus,
+                   &def->info.addr.pci.slot) < 3) {
+            virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                                 _("Unable to parse devaddr parameter '%s'"),
+                                 devaddr);
+            goto error;
+        }
+        def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+    } else {
+        if (virDomainDeviceInfoParseXML(conn, node, &def->info, 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->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+        def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+                             _("Network interfaces must use 'pci' address type"));
         goto error;
     }
 
@@ -2321,83 +2564,27 @@ 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")) {
+                /* Legacy back-compat. Don't add any more attributes here */
                 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->info.addr.pci.domain,
+                           &def->info.addr.pci.bus,
+                           &def->info.addr.pci.slot) < 3) {
                     virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
                                          _("Unable to parse devaddr parameter '%s'"),
                                          devaddr);
                     VIR_FREE(devaddr);
                     goto out;
                 }
+                def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
             } else {
                 virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
                                      _("unknown pci source type '%s'"),
@@ -2475,14 +2662,29 @@ virDomainHostdevDefParseXML(virConnectPtr conn,
                         if (virDomainHostdevSubsysPciDefParseXML(conn, cur, def, flags) < 0)
                             goto error;
                 }
-            } else {
-                virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                                     _("unknown node %s"), cur->name);
             }
         }
         cur = cur->next;
     }
 
+    if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+        if (virDomainDeviceInfoParseXML(conn, node, &def->info, flags) < 0)
+            goto error;
+    }
+
+    if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
+        switch (def->source.subsys.type) {
+        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
+            if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+                def->info.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);
@@ -3869,8 +4071,7 @@ virDomainLifecycleDefFormat(virConnectPtr conn,
 static int
 virDomainDiskDefFormat(virConnectPtr conn,
                        virBufferPtr buf,
-                       virDomainDiskDefPtr def,
-                       int flags)
+                       virDomainDiskDefPtr def)
 {
     const char *type = virDomainDiskTypeToString(def->type);
     const char *device = virDomainDiskDeviceTypeToString(def->device);
@@ -3947,15 +4148,8 @@ 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 (virDomainDeviceInfoFormat(buf, &def->info) < 0)
+        return -1;
 
     virBufferAddLit(buf, "    </disk>\n");
 
@@ -4089,21 +4283,18 @@ virDomainNetDefFormat(virConnectPtr conn,
                               def->model);
 
     if (flags & VIR_DOMAIN_XML_INTERNAL_STATUS) {
+        /* Legacy back-compat. Don't add any more attributes here */
         virBufferAddLit(buf, "      <state");
-        if (def->nic_name)
-            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 (virDomainDeviceInfoFormat(buf, &def->info) < 0)
+        return -1;
+
     virBufferAddLit(buf, "    </interface>\n");
 
     return 0;
@@ -4477,8 +4668,7 @@ virDomainGraphicsDefFormat(virConnectPtr conn,
 static int
 virDomainHostdevDefFormat(virConnectPtr conn,
                           virBufferPtr buf,
-                          virDomainHostdevDefPtr def,
-                          int flags)
+                          virDomainHostdevDefPtr def)
 {
     const char *mode = virDomainHostdevModeTypeToString(def->mode);
     const char *type;
@@ -4512,25 +4702,19 @@ 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 (virDomainDeviceInfoFormat(buf, &def->info) < 0)
+        return -1;
+
     virBufferAddLit(buf, "    </hostdev>\n");
 
     return 0;
@@ -4695,7 +4879,7 @@ char *virDomainDefFormat(virConnectPtr conn,
                               def->emulator);
 
     for (n = 0 ; n < def->ndisks ; n++)
-        if (virDomainDiskDefFormat(conn, &buf, def->disks[n], flags) < 0)
+        if (virDomainDiskDefFormat(conn, &buf, def->disks[n]) < 0)
             goto cleanup;
 
     for (n = 0 ; n < def->nfss ; n++)
@@ -4763,7 +4947,7 @@ char *virDomainDefFormat(virConnectPtr conn,
             goto cleanup;
 
     for (n = 0 ; n < def->nhostdevs ; n++)
-        if (virDomainHostdevDefFormat(conn, &buf, def->hostdevs[n], flags) < 0)
+        if (virDomainHostdevDefFormat(conn, &buf, def->hostdevs[n]) < 0)
             goto cleanup;
 
     if (def->watchdog)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a807e9d43c..c50aae0ec9 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -64,6 +64,32 @@ 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
+};
+
+typedef struct _virDomainDevicePCIAddress virDomainDevicePCIAddress;
+typedef virDomainDevicePCIAddress *virDomainDevicePCIAddressPtr;
+struct _virDomainDevicePCIAddress {
+    unsigned int domain;
+    unsigned int bus;
+    unsigned int slot;
+    unsigned int function;
+};
+
+typedef struct _virDomainDeviceInfo virDomainDeviceInfo;
+typedef virDomainDeviceInfo *virDomainDeviceInfoPtr;
+struct _virDomainDeviceInfo {
+    int type;
+    union {
+        virDomainDevicePCIAddress pci;
+    } addr;
+};
+
+
 /* Two types of disk backends */
 enum virDomainDiskType {
     VIR_DOMAIN_DISK_TYPE_BLOCK,
@@ -119,20 +145,10 @@ struct _virDomainDiskDef {
     int cachemode;
     unsigned int readonly : 1;
     unsigned int shared : 1;
-    struct {
-        unsigned domain;
-        unsigned bus;
-        unsigned slot;
-    } pci_addr;
+    virDomainDeviceInfo info;
     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 {
@@ -199,22 +215,15 @@ struct _virDomainNetDef {
         } internal;
     } data;
     char *ifname;
+    virDomainDeviceInfo info;
+    /* XXX figure out how to remove this */
     char *nic_name;
+    /* XXX figure out how to remove this */
     char *hostnet_name;
-    struct {
-        unsigned domain;
-        unsigned bus;
-        unsigned slot;
-    } pci_addr;
+    /* XXX figure out how to remove this */
     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,
@@ -442,17 +451,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 {
@@ -463,16 +462,9 @@ struct _virDomainHostdevDef {
         } caps;
     } source;
     char* target;
+    virDomainDeviceInfo info; /* 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,
@@ -673,6 +665,7 @@ virDomainObjIsActive(virDomainObjPtr dom)
     return dom->def->id != -1;
 }
 
+
 int virDomainObjListInit(virDomainObjListPtr objs);
 void virDomainObjListDeinit(virDomainObjListPtr objs);
 
@@ -695,6 +688,13 @@ void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def);
 void virDomainVideoDefFree(virDomainVideoDefPtr def);
 void virDomainHostdevDefFree(virDomainHostdevDefPtr def);
 void virDomainDeviceDefFree(virDomainDeviceDefPtr def);
+int virDomainDevicePCIAddressEqual(virDomainDevicePCIAddressPtr a,
+                                   virDomainDevicePCIAddressPtr b);
+int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info,
+                                  int type);
+int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr);
+int virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info);
+void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info);
 void virDomainDefFree(virDomainDefPtr vm);
 void virDomainObjRef(virDomainObjPtr vm);
 /* Returns 1 if the object was freed, 0 if more refs exist */
@@ -832,6 +832,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 a8947757b1..2f6d465b4d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -174,6 +174,12 @@ virDomainObjListInit;
 virDomainObjListDeinit;
 virDomainObjRef;
 virDomainObjUnref;
+virDomainDeviceAddressEqual;
+virDomainDevicePCIAddressEqual;
+virDomainDeviceAddressIsValid;
+virDomainDevicePCIAddressIsValid;
+virDomainDeviceInfoIsSet;
+virDomainDeviceAddressClear;
 
 
 # domain_event.h
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f03ce91521..7517075bac 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5111,13 +5111,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->info.addr.pci.domain,
+                                &dev->data.disk->info.addr.pci.bus,
+                                &dev->data.disk->info.addr.pci.slot);
     qemuDomainObjExitMonitorWithDriver(driver, vm);
 
-    if (ret == 0)
+    if (ret == 0) {
+        dev->data.disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
         virDomainDiskInsertPreAlloced(vm->def, dev->data.disk);
+    }
 
     return ret;
 }
@@ -5240,12 +5242,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->info.addr.pci.domain,
+                                 &net->info.addr.pci.bus,
+                                 &net->info.addr.pci.slot) < 0) {
         qemuDomainObjExitMonitorWithDriver(driver, vm);
         goto try_remove;
     }
+    net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
     qemuDomainObjExitMonitorWithDriver(driver, vm);
 
     ret = 0;
@@ -5328,12 +5331,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->info.addr.pci.domain,
+                                      &hostdev->info.addr.pci.bus,
+                                      &hostdev->info.addr.pci.slot);
     qemuDomainObjExitMonitorWithDriver(driver, vm);
     if (ret < 0)
         goto error;
+    hostdev->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
 
     vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
 
@@ -5563,18 +5567,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->info,
+                                       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->info.addr.pci.domain,
+                                   detach->info.addr.pci.bus,
+                                   detach->info.addr.pci.slot) < 0) {
         qemuDomainObjExitMonitor(vm);
         goto cleanup;
     }
@@ -5629,7 +5633,14 @@ qemudDomainDetachNetDevice(virConnectPtr conn,
         goto cleanup;
     }
 
-    if (!virNetHasValidPciAddr(detach) || detach->vlan < 0 || !detach->hostnet_name) {
+    if (!virDomainDeviceAddressIsValid(&detach->info,
+                                       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;
@@ -5637,9 +5648,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->info.addr.pci.domain,
+                                   detach->info.addr.pci.bus,
+                                   detach->info.addr.pci.slot) < 0) {
         qemuDomainObjExitMonitorWithDriver(driver, vm);
         goto cleanup;
     }
@@ -5717,17 +5728,18 @@ static int qemudDomainDetachHostPciDevice(virConnectPtr conn,
         return -1;
     }
 
-    if (!virHostdevHasValidGuestAddr(detach)) {
+    if (!virDomainDeviceAddressIsValid(&detach->info,
+                                       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->info.addr.pci.domain,
+                                   detach->info.addr.pci.bus,
+                                   detach->info.addr.pci.slot) < 0) {
         qemuDomainObjExitMonitorWithDriver(driver, vm);
         return -1;
     }