Not all device types were represented in virDomainDeviceType, so some types of devices couldn't be represented in a virDomainDeviceDef (which requires a different type of pointer in the union for each different kind of device). Since serial, parallel, channel, and console devices are all virDomainChrDef, and the virDomainDeviceType is never used to produce a string from the type (and only used in the other direction internally to code, never to produce XML), I only added one "CHR" type, which is associated with "virDomainChrDefPtr chr" in the union. --- New patch in V2. src/conf/domain_conf.c | 6 +++++- src/conf/domain_conf.h | 11 +++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f9654f1..26190f1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -123,6 +123,7 @@ VIR_ENUM_IMPL(virDomainLifecycleCrash, VIR_DOMAIN_LIFECYCLE_CRASH_LAST, "coredump-restart") VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, + "none", "disk", "lease", "filesystem", @@ -135,7 +136,10 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "controller", "graphics", "hub", - "redirdev") + "redirdev", + "smartcard", + "chr", + "memballoon") VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, "none", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 596be4d..9acf1e1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1179,7 +1179,8 @@ enum virDomainSmbiosMode { }; /* Flags for the 'type' field in next struct */ -enum virDomainDeviceType { +typedef enum { + VIR_DOMAIN_DEVICE_NONE = 0, VIR_DOMAIN_DEVICE_DISK, VIR_DOMAIN_DEVICE_LEASE, VIR_DOMAIN_DEVICE_FS, @@ -1193,9 +1194,12 @@ enum virDomainDeviceType { VIR_DOMAIN_DEVICE_GRAPHICS, VIR_DOMAIN_DEVICE_HUB, VIR_DOMAIN_DEVICE_REDIRDEV, + VIR_DOMAIN_DEVICE_SMARTCARD, + VIR_DOMAIN_DEVICE_CHR, + VIR_DOMAIN_DEVICE_MEMBALLOON, VIR_DOMAIN_DEVICE_LAST, -}; +} virDomainDeviceType; typedef struct _virDomainDeviceDef virDomainDeviceDef; typedef virDomainDeviceDef *virDomainDeviceDefPtr; @@ -1215,6 +1219,9 @@ struct _virDomainDeviceDef { virDomainGraphicsDefPtr graphics; virDomainHubDefPtr hub; virDomainRedirdevDefPtr redirdev; + virDomainSmartcardDefPtr smartcard; + virDomainChrDefPtr chr; + virDomainMemballoonDefPtr memballoon; } data; }; -- 1.7.7.6

This patch is only code movement + adding some forward definitions of typedefs. virDomainHostdevDef (not just a pointer to it, but an actual object) will be needed in virDomainNetDef and virDomainActualNetDef, so it must be relocated earlier in the file. Likewise, virDomainDeviceDef will be needed in virDomainHostdevDef, so it must be moved up even earlier. This, in turn, creates a forward reference problem, but fortunately only with pointers to other device types, so their typedefs can be moved up in the file, eliminating the problem. --- V2: aside from the fact that there are now more types of device to have their definitions/enums relocated, I learned that older versions of gcc will not allow a duplicate typedef, even if it is identical to the original, so rather than just copying the device typedefs from their original locations just above each struct definition, I had to move them, removing the original. src/conf/domain_conf.h | 262 ++++++++++++++++++++++++++---------------------- 1 files changed, 141 insertions(+), 121 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9acf1e1..68b7cfd 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -44,6 +44,104 @@ # include "virnetdevopenvswitch.h" # include "virnetdevbandwidth.h" +/* forward declarations of all device types, required by + * virDomainDeviceDef + */ +typedef struct _virDomainDiskDef virDomainDiskDef; +typedef virDomainDiskDef *virDomainDiskDefPtr; + +typedef struct _virDomainControllerDef virDomainControllerDef; +typedef virDomainControllerDef *virDomainControllerDefPtr; + +typedef struct _virDomainLeaseDef virDomainLeaseDef; +typedef virDomainLeaseDef *virDomainLeaseDefPtr; + +typedef struct _virDomainFSDef virDomainFSDef; +typedef virDomainFSDef *virDomainFSDefPtr; + +typedef struct _virDomainNetDef virDomainNetDef; +typedef virDomainNetDef *virDomainNetDefPtr; + +typedef struct _virDomainInputDef virDomainInputDef; +typedef virDomainInputDef *virDomainInputDefPtr; + +typedef struct _virDomainSoundDef virDomainSoundDef; +typedef virDomainSoundDef *virDomainSoundDefPtr; + +typedef struct _virDomainVideoDef virDomainVideoDef; +typedef virDomainVideoDef *virDomainVideoDefPtr; + +typedef struct _virDomainHostdevDef virDomainHostdevDef; +typedef virDomainHostdevDef *virDomainHostdevDefPtr; + +typedef struct _virDomainWatchdogDef virDomainWatchdogDef; +typedef virDomainWatchdogDef *virDomainWatchdogDefPtr; + +typedef struct _virDomainGraphicsDef virDomainGraphicsDef; +typedef virDomainGraphicsDef *virDomainGraphicsDefPtr; + +typedef struct _virDomainHubDef virDomainHubDef; +typedef virDomainHubDef *virDomainHubDefPtr; + +typedef struct _virDomainRedirdevDef virDomainRedirdevDef; +typedef virDomainRedirdevDef *virDomainRedirdevDefPtr; + +typedef struct _virDomainSmartcardDef virDomainSmartcardDef; +typedef virDomainSmartcardDef *virDomainSmartcardDefPtr; + +typedef struct _virDomainChrDef virDomainChrDef; +typedef virDomainChrDef *virDomainChrDefPtr; + +typedef struct _virDomainMemballoonDef virDomainMemballoonDef; +typedef virDomainMemballoonDef *virDomainMemballoonDefPtr; + +/* Flags for the 'type' field in virDomainDeviceDef */ +typedef enum { + VIR_DOMAIN_DEVICE_NONE = 0, + VIR_DOMAIN_DEVICE_DISK, + VIR_DOMAIN_DEVICE_LEASE, + VIR_DOMAIN_DEVICE_FS, + VIR_DOMAIN_DEVICE_NET, + VIR_DOMAIN_DEVICE_INPUT, + VIR_DOMAIN_DEVICE_SOUND, + VIR_DOMAIN_DEVICE_VIDEO, + VIR_DOMAIN_DEVICE_HOSTDEV, + VIR_DOMAIN_DEVICE_WATCHDOG, + VIR_DOMAIN_DEVICE_CONTROLLER, + VIR_DOMAIN_DEVICE_GRAPHICS, + VIR_DOMAIN_DEVICE_HUB, + VIR_DOMAIN_DEVICE_REDIRDEV, + VIR_DOMAIN_DEVICE_SMARTCARD, + VIR_DOMAIN_DEVICE_CHR, + VIR_DOMAIN_DEVICE_MEMBALLOON, + + VIR_DOMAIN_DEVICE_LAST, +} virDomainDeviceType; + +typedef struct _virDomainDeviceDef virDomainDeviceDef; +typedef virDomainDeviceDef *virDomainDeviceDefPtr; +struct _virDomainDeviceDef { + int type; /* enum virDomainDeviceType */ + union { + virDomainDiskDefPtr disk; + virDomainControllerDefPtr controller; + virDomainLeaseDefPtr lease; + virDomainFSDefPtr fs; + virDomainNetDefPtr net; + virDomainInputDefPtr input; + virDomainSoundDefPtr sound; + virDomainVideoDefPtr video; + virDomainHostdevDefPtr hostdev; + virDomainWatchdogDefPtr watchdog; + virDomainGraphicsDefPtr graphics; + virDomainHubDefPtr hub; + virDomainRedirdevDefPtr redirdev; + virDomainSmartcardDefPtr smartcard; + virDomainChrDefPtr chr; + virDomainMemballoonDefPtr memballoon; + } data; +}; + /* Different types of hypervisor */ /* NB: Keep in sync with virDomainVirtTypeToString impl */ enum virDomainVirtType { @@ -234,8 +332,6 @@ struct _virDomainHostdevOrigStates { } states; }; -typedef struct _virDomainLeaseDef virDomainLeaseDef; -typedef virDomainLeaseDef *virDomainLeaseDefPtr; struct _virDomainLeaseDef { char *lockspace; char *key; @@ -244,6 +340,49 @@ struct _virDomainLeaseDef { }; +enum virDomainHostdevMode { + VIR_DOMAIN_HOSTDEV_MODE_SUBSYS, + VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES, + + VIR_DOMAIN_HOSTDEV_MODE_LAST, +}; + +enum virDomainHostdevSubsysType { + VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB, + VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI, + + VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST +}; + +/* basic device for direct passthrough */ +struct _virDomainHostdevDef { + int mode; /* enum virDomainHostdevMode */ + unsigned int managed : 1; + union { + struct { + int type; /* enum virDomainHostdevBusType */ + union { + struct { + unsigned bus; + unsigned device; + + unsigned vendor; + unsigned product; + } usb; + virDomainDevicePCIAddress pci; /* host address */ + } u; + } subsys; + struct { + /* TBD: struct capabilities see: + * https://www.redhat.com/archives/libvir-list/2008-July/msg00429.html + */ + int dummy; + } caps; + } source; + virDomainHostdevOrigStates origstates; + virDomainDeviceInfo info; /* Guest address */ +}; + /* Two types of disk backends */ enum virDomainDiskType { VIR_DOMAIN_DISK_TYPE_BLOCK, @@ -389,8 +528,6 @@ struct _virDomainBlockIoTuneInfo { typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr; /* Stores the virtual disk configuration */ -typedef struct _virDomainDiskDef virDomainDiskDef; -typedef virDomainDiskDef *virDomainDiskDefPtr; struct _virDomainDiskDef { int type; int device; @@ -481,8 +618,6 @@ struct _virDomainVirtioSerialOpts { }; /* Stores the virtual disk controller configuration */ -typedef struct _virDomainControllerDef virDomainControllerDef; -typedef virDomainControllerDef *virDomainControllerDefPtr; struct _virDomainControllerDef { int type; int idx; @@ -530,8 +665,6 @@ enum virDomainFSWrpolicy { VIR_DOMAIN_FS_WRPOLICY_LAST }; -typedef struct _virDomainFSDef virDomainFSDef; -typedef virDomainFSDef *virDomainFSDefPtr; struct _virDomainFSDef { int type; int fsdriver; @@ -610,8 +743,6 @@ struct _virDomainActualNetDef { }; /* Stores the virtual network interface configuration */ -typedef struct _virDomainNetDef virDomainNetDef; -typedef virDomainNetDef *virDomainNetDefPtr; struct _virDomainNetDef { enum virDomainNetType type; unsigned char mac[VIR_MAC_BUFLEN]; @@ -769,8 +900,6 @@ struct _virDomainChrSourceDef { }; /* A complete character device, both host and domain views. */ -typedef struct _virDomainChrDef virDomainChrDef; -typedef virDomainChrDef *virDomainChrDefPtr; struct _virDomainChrDef { int deviceType; int targetType; @@ -796,8 +925,6 @@ enum virDomainSmartcardType { # define VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES 3 # define VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE "/etc/pki/nssdb" -typedef struct _virDomainSmartcardDef virDomainSmartcardDef; -typedef virDomainSmartcardDef *virDomainSmartcardDefPtr; struct _virDomainSmartcardDef { int type; /* virDomainSmartcardType */ union { @@ -812,8 +939,6 @@ struct _virDomainSmartcardDef { virDomainDeviceInfo info; }; -typedef struct _virDomainHubDef virDomainHubDef; -typedef virDomainHubDef *virDomainHubDefPtr; struct _virDomainHubDef { int type; virDomainDeviceInfo info; @@ -834,8 +959,6 @@ enum virDomainInputBus { VIR_DOMAIN_INPUT_BUS_LAST }; -typedef struct _virDomainInputDef virDomainInputDef; -typedef virDomainInputDef *virDomainInputDefPtr; struct _virDomainInputDef { int type; int bus; @@ -852,8 +975,6 @@ enum virDomainSoundModel { VIR_DOMAIN_SOUND_MODEL_LAST }; -typedef struct _virDomainSoundDef virDomainSoundDef; -typedef virDomainSoundDef *virDomainSoundDefPtr; struct _virDomainSoundDef { int model; virDomainDeviceInfo info; @@ -877,8 +998,6 @@ enum virDomainWatchdogAction { VIR_DOMAIN_WATCHDOG_ACTION_LAST }; -typedef struct _virDomainWatchdogDef virDomainWatchdogDef; -typedef virDomainWatchdogDef *virDomainWatchdogDefPtr; struct _virDomainWatchdogDef { int model; int action; @@ -906,8 +1025,6 @@ struct _virDomainVideoAccelDef { }; -typedef struct _virDomainVideoDef virDomainVideoDef; -typedef virDomainVideoDef *virDomainVideoDefPtr; struct _virDomainVideoDef { int type; unsigned int vram; @@ -1042,8 +1159,6 @@ struct _virDomainGraphicsListenDef { char *network; }; -typedef struct _virDomainGraphicsDef virDomainGraphicsDef; -typedef virDomainGraphicsDef *virDomainGraphicsDefPtr; struct _virDomainGraphicsDef { int type; union { @@ -1091,58 +1206,12 @@ struct _virDomainGraphicsDef { virDomainGraphicsListenDefPtr listens; }; -enum virDomainHostdevMode { - VIR_DOMAIN_HOSTDEV_MODE_SUBSYS, - VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES, - - VIR_DOMAIN_HOSTDEV_MODE_LAST, -}; - -enum virDomainHostdevSubsysType { - VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB, - VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI, - - VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST -}; - -typedef struct _virDomainHostdevDef virDomainHostdevDef; -typedef virDomainHostdevDef *virDomainHostdevDefPtr; -struct _virDomainHostdevDef { - int mode; /* enum virDomainHostdevMode */ - unsigned int managed : 1; - union { - struct { - int type; /* enum virDomainHostdevBusType */ - union { - struct { - unsigned bus; - unsigned device; - - unsigned vendor; - unsigned product; - } usb; - virDomainDevicePCIAddress pci; /* host address */ - } u; - } subsys; - struct { - /* TBD: struct capabilities see: - * https://www.redhat.com/archives/libvir-list/2008-July/msg00429.html - */ - int dummy; - } caps; - } source; - virDomainDeviceInfo info; /* Guest address */ - virDomainHostdevOrigStates origstates; -}; - enum virDomainRedirdevBus { VIR_DOMAIN_REDIRDEV_BUS_USB, VIR_DOMAIN_REDIRDEV_BUS_LAST }; -typedef struct _virDomainRedirdevDef virDomainRedirdevDef; -typedef virDomainRedirdevDef *virDomainRedirdevDefPtr; struct _virDomainRedirdevDef { int bus; /* enum virDomainRedirdevBus */ @@ -1161,8 +1230,6 @@ enum { VIR_DOMAIN_MEMBALLOON_MODEL_LAST }; -typedef struct _virDomainMemballoonDef virDomainMemballoonDef; -typedef virDomainMemballoonDef *virDomainMemballoonDefPtr; struct _virDomainMemballoonDef { int model; virDomainDeviceInfo info; @@ -1178,53 +1245,6 @@ enum virDomainSmbiosMode { VIR_DOMAIN_SMBIOS_LAST }; -/* Flags for the 'type' field in next struct */ -typedef enum { - VIR_DOMAIN_DEVICE_NONE = 0, - VIR_DOMAIN_DEVICE_DISK, - VIR_DOMAIN_DEVICE_LEASE, - VIR_DOMAIN_DEVICE_FS, - VIR_DOMAIN_DEVICE_NET, - VIR_DOMAIN_DEVICE_INPUT, - VIR_DOMAIN_DEVICE_SOUND, - VIR_DOMAIN_DEVICE_VIDEO, - VIR_DOMAIN_DEVICE_HOSTDEV, - VIR_DOMAIN_DEVICE_WATCHDOG, - VIR_DOMAIN_DEVICE_CONTROLLER, - VIR_DOMAIN_DEVICE_GRAPHICS, - VIR_DOMAIN_DEVICE_HUB, - VIR_DOMAIN_DEVICE_REDIRDEV, - VIR_DOMAIN_DEVICE_SMARTCARD, - VIR_DOMAIN_DEVICE_CHR, - VIR_DOMAIN_DEVICE_MEMBALLOON, - - VIR_DOMAIN_DEVICE_LAST, -} virDomainDeviceType; - -typedef struct _virDomainDeviceDef virDomainDeviceDef; -typedef virDomainDeviceDef *virDomainDeviceDefPtr; -struct _virDomainDeviceDef { - int type; - union { - virDomainDiskDefPtr disk; - virDomainControllerDefPtr controller; - virDomainLeaseDefPtr lease; - virDomainFSDefPtr fs; - virDomainNetDefPtr net; - virDomainInputDefPtr input; - virDomainSoundDefPtr sound; - virDomainVideoDefPtr video; - virDomainHostdevDefPtr hostdev; - virDomainWatchdogDefPtr watchdog; - virDomainGraphicsDefPtr graphics; - virDomainHubDefPtr hub; - virDomainRedirdevDefPtr redirdev; - virDomainSmartcardDefPtr smartcard; - virDomainChrDefPtr chr; - virDomainMemballoonDefPtr memballoon; - } data; -}; - # define VIR_DOMAIN_MAX_BOOT_DEVS 4 -- 1.7.7.6

No code change, movement only. This is necessary to eliminate forward references. --- V2: No change src/conf/domain_conf.c | 417 ++++++++++++++++++++++++------------------------ 1 files changed, 208 insertions(+), 209 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 26190f1..a2536de 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2515,6 +2515,214 @@ virDomainParseLegacyDeviceAddress(char *devaddr, return 0; } +static int +virDomainHostdevSubsysUsbDefParseXML(const xmlNodePtr node, + virDomainHostdevDefPtr def) +{ + + int ret = -1; + int got_product, got_vendor; + xmlNodePtr cur; + + /* Product can validly be 0, so we need some extra help to determine + * if it is uninitialized*/ + got_product = 0; + got_vendor = 0; + + cur = node->children; + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE) { + if (xmlStrEqual(cur->name, BAD_CAST "vendor")) { + char *vendor = virXMLPropString(cur, "id"); + + if (vendor) { + got_vendor = 1; + if (virStrToLong_ui(vendor, NULL, 0, + &def->source.subsys.u.usb.vendor) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse vendor id %s"), vendor); + VIR_FREE(vendor); + goto out; + } + VIR_FREE(vendor); + } else { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("usb vendor needs id")); + goto out; + } + } else if (xmlStrEqual(cur->name, BAD_CAST "product")) { + char* product = virXMLPropString(cur, "id"); + + if (product) { + got_product = 1; + if (virStrToLong_ui(product, NULL, 0, + &def->source.subsys.u.usb.product) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse product %s"), + product); + VIR_FREE(product); + goto out; + } + VIR_FREE(product); + } else { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("usb product needs id")); + goto out; + } + } else if (xmlStrEqual(cur->name, BAD_CAST "address")) { + char *bus, *device; + + bus = virXMLPropString(cur, "bus"); + if (bus) { + if (virStrToLong_ui(bus, NULL, 0, + &def->source.subsys.u.usb.bus) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse bus %s"), bus); + VIR_FREE(bus); + goto out; + } + VIR_FREE(bus); + } else { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("usb address needs bus id")); + goto out; + } + + device = virXMLPropString(cur, "device"); + if (device) { + if (virStrToLong_ui(device, NULL, 0, + &def->source.subsys.u.usb.device) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse device %s"), + device); + VIR_FREE(device); + goto out; + } + VIR_FREE(device); + } else { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("usb address needs device id")); + goto out; + } + } else { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown usb source type '%s'"), + cur->name); + goto out; + } + } + cur = cur->next; + } + + if (got_vendor && def->source.subsys.u.usb.vendor == 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("vendor cannot be 0.")); + goto out; + } + + if (!got_vendor && got_product) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("missing vendor")); + goto out; + } + if (got_vendor && !got_product) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("missing product")); + goto out; + } + + ret = 0; +out: + return ret; +} + +/* The internal XML for host PCI device's original states: + * + * <origstates> + * <unbind/> + * <removeslot/> + * <reprobe/> + * </origstates> + */ +static int +virDomainHostdevSubsysPciOrigStatesDefParseXML(const xmlNodePtr node, + virDomainHostdevOrigStatesPtr def) +{ + xmlNodePtr cur; + cur = node->children; + + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE) { + if (xmlStrEqual(cur->name, BAD_CAST "unbind")) { + def->states.pci.unbind_from_stub = 1; + } else if (xmlStrEqual(cur->name, BAD_CAST "removeslot")) { + def->states.pci.remove_slot = 1; + } else if (xmlStrEqual(cur->name, BAD_CAST "reprobe")) { + def->states.pci.reprobe = 1; + } else { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported element '%s' of 'origstates'"), + cur->name); + return -1; + } + } + cur = cur->next; + } + + return 0; +} + +static int +virDomainHostdevSubsysPciDefParseXML(const xmlNodePtr node, + virDomainHostdevDefPtr def, + unsigned int flags) +{ + int ret = -1; + xmlNodePtr cur; + + cur = node->children; + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE) { + if (xmlStrEqual(cur->name, BAD_CAST "address")) { + virDomainDevicePCIAddressPtr addr = + &def->source.subsys.u.pci; + + if (virDomainDevicePCIAddressParseXML(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 && + virDomainParseLegacyDeviceAddress(devaddr, + &def->info.addr.pci) < 0) { + virDomainReportError(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 if ((flags & VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES) && + xmlStrEqual(cur->name, BAD_CAST "origstates")) { + virDomainHostdevOrigStatesPtr states = &def->origstates; + if (virDomainHostdevSubsysPciOrigStatesDefParseXML(cur, states) < 0) + goto out; + } else { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown pci source type '%s'"), + cur->name); + goto out; + } + } + cur = cur->next; + } + + ret = 0; +out: + return ret; +} + int virDomainDiskFindControllerModel(virDomainDefPtr def, virDomainDiskDefPtr disk, @@ -6114,215 +6322,6 @@ error: return NULL; } -static int -virDomainHostdevSubsysUsbDefParseXML(const xmlNodePtr node, - virDomainHostdevDefPtr def) -{ - - int ret = -1; - int got_product, got_vendor; - xmlNodePtr cur; - - /* Product can validly be 0, so we need some extra help to determine - * if it is uninitialized*/ - got_product = 0; - got_vendor = 0; - - cur = node->children; - while (cur != NULL) { - if (cur->type == XML_ELEMENT_NODE) { - if (xmlStrEqual(cur->name, BAD_CAST "vendor")) { - char *vendor = virXMLPropString(cur, "id"); - - if (vendor) { - got_vendor = 1; - if (virStrToLong_ui(vendor, NULL, 0, - &def->source.subsys.u.usb.vendor) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse vendor id %s"), vendor); - VIR_FREE(vendor); - goto out; - } - VIR_FREE(vendor); - } else { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("usb vendor needs id")); - goto out; - } - } else if (xmlStrEqual(cur->name, BAD_CAST "product")) { - char* product = virXMLPropString(cur, "id"); - - if (product) { - got_product = 1; - if (virStrToLong_ui(product, NULL, 0, - &def->source.subsys.u.usb.product) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse product %s"), - product); - VIR_FREE(product); - goto out; - } - VIR_FREE(product); - } else { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("usb product needs id")); - goto out; - } - } else if (xmlStrEqual(cur->name, BAD_CAST "address")) { - char *bus, *device; - - bus = virXMLPropString(cur, "bus"); - if (bus) { - if (virStrToLong_ui(bus, NULL, 0, - &def->source.subsys.u.usb.bus) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse bus %s"), bus); - VIR_FREE(bus); - goto out; - } - VIR_FREE(bus); - } else { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("usb address needs bus id")); - goto out; - } - - device = virXMLPropString(cur, "device"); - if (device) { - if (virStrToLong_ui(device, NULL, 0, - &def->source.subsys.u.usb.device) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse device %s"), - device); - VIR_FREE(device); - goto out; - } - VIR_FREE(device); - } else { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("usb address needs device id")); - goto out; - } - } else { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown usb source type '%s'"), - cur->name); - goto out; - } - } - cur = cur->next; - } - - if (got_vendor && def->source.subsys.u.usb.vendor == 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("vendor cannot be 0.")); - goto out; - } - - if (!got_vendor && got_product) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing vendor")); - goto out; - } - if (got_vendor && !got_product) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing product")); - goto out; - } - - ret = 0; -out: - return ret; -} - -/* The internal XML for host PCI device's original states: - * - * <origstates> - * <unbind/> - * <removeslot/> - * <reprobe/> - * </origstates> - */ -static int -virDomainHostdevSubsysPciOrigStatesDefParseXML(const xmlNodePtr node, - virDomainHostdevOrigStatesPtr def) -{ - xmlNodePtr cur; - cur = node->children; - - while (cur != NULL) { - if (cur->type == XML_ELEMENT_NODE) { - if (xmlStrEqual(cur->name, BAD_CAST "unbind")) { - def->states.pci.unbind_from_stub = 1; - } else if (xmlStrEqual(cur->name, BAD_CAST "removeslot")) { - def->states.pci.remove_slot = 1; - } else if (xmlStrEqual(cur->name, BAD_CAST "reprobe")) { - def->states.pci.reprobe = 1; - } else { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported element '%s' of 'origstates'"), - cur->name); - return -1; - } - } - cur = cur->next; - } - - return 0; -} - -static int -virDomainHostdevSubsysPciDefParseXML(const xmlNodePtr node, - virDomainHostdevDefPtr def, - unsigned int flags) -{ - int ret = -1; - xmlNodePtr cur; - - cur = node->children; - while (cur != NULL) { - if (cur->type == XML_ELEMENT_NODE) { - if (xmlStrEqual(cur->name, BAD_CAST "address")) { - virDomainDevicePCIAddressPtr addr = - &def->source.subsys.u.pci; - - if (virDomainDevicePCIAddressParseXML(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 && - virDomainParseLegacyDeviceAddress(devaddr, - &def->info.addr.pci) < 0) { - virDomainReportError(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 if ((flags & VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES) && - xmlStrEqual(cur->name, BAD_CAST "origstates")) { - virDomainHostdevOrigStatesPtr states = &def->origstates; - if (virDomainHostdevSubsysPciOrigStatesDefParseXML(cur, states) < 0) - goto out; - } else { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown pci source type '%s'"), - cur->name); - goto out; - } - } - cur = cur->next; - } - - ret = 0; -out: - return ret; -} - - static virDomainHostdevDefPtr virDomainHostdevDefParseXML(const xmlNodePtr node, virBitmapPtr bootMap, -- 1.7.7.6

The virDomainDeviceInfoPtrs in qemuCollectPCIAddress and qemuComparePCIDevice are named "dev" and "dev1", but those functions will be changed (in order to match a change in the args sent to virDomainDeviceInfoIterate() callback args) to contain a virDomainDeviceDefPtr device. This patch renames "dev" to "info" (and "dev[n]" to "info[n]") to avoid later confusion. --- new Patch in V2. src/qemu/qemu_command.c | 18 +++++++++--------- src/qemu/qemu_hotplug.c | 12 ++++++------ 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 01adf0d..04e580f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -835,22 +835,22 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev) static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, - virDomainDeviceInfoPtr dev, + virDomainDeviceInfoPtr info, void *opaque) { int ret = -1; char *addr = NULL; qemuDomainPCIAddressSetPtr addrs = opaque; - if (dev->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) return 0; - addr = qemuPCIAddressAsString(dev); + addr = qemuPCIAddressAsString(info); if (!addr) goto cleanup; if (virHashLookup(addrs->used, addr)) { - if (dev->addr.pci.function != 0) { + if (info->addr.pci.function != 0) { qemuReportError(VIR_ERR_XML_ERROR, _("Attempted double use of PCI Address '%s' " "(may need \"multifunction='on'\" for device on function 0"), @@ -867,15 +867,15 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, goto cleanup; addr = NULL; - if ((dev->addr.pci.function == 0) && - (dev->addr.pci.multi != VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_ON)) { + if ((info->addr.pci.function == 0) && + (info->addr.pci.multi != VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_ON)) { /* a function 0 w/o multifunction=on must reserve the entire slot */ int function; - virDomainDeviceInfo temp_dev = *dev; + virDomainDeviceInfo temp_info = *info; for (function = 1; function < QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) { - temp_dev.addr.pci.function = function; - addr = qemuPCIAddressAsString(&temp_dev); + temp_info.addr.pci.function = function; + addr = qemuPCIAddressAsString(&temp_info); if (!addr) goto cleanup; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5c64dbe..0ca3186 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1481,17 +1481,17 @@ static inline int qemuFindDisk(virDomainDefPtr def, const char *dst) } static int qemuComparePCIDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, - virDomainDeviceInfoPtr dev1, + virDomainDeviceInfoPtr info1, void *opaque) { - virDomainDeviceInfoPtr dev2 = opaque; + virDomainDeviceInfoPtr info2 = opaque; - if (dev1->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI || - dev2->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + if (info1->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI || + info2->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) return 0; - if (dev1->addr.pci.slot == dev2->addr.pci.slot && - dev1->addr.pci.function != dev2->addr.pci.function) + if (info1->addr.pci.slot == info2->addr.pci.slot && + info1->addr.pci.function != info2->addr.pci.function) return -1; return 0; } -- 1.7.7.6

There will be cases where the iterator callback will need to know the type of the device whose info is being operated on, and possibly even need to use some of the device's config. This patch adds a virDomainDeviceDefPtr to the args of every callback, and fills it in appropriately as the devices are iterated through. --- New patch in V2. src/conf/domain_conf.c | 112 +++++++++++++++++++++++++++++++++------------- src/conf/domain_conf.h | 3 +- src/qemu/qemu_command.c | 2 + src/qemu/qemu_hotplug.c | 1 + 4 files changed, 85 insertions(+), 33 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a2536de..03f8564 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1829,6 +1829,7 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info) static int virDomainDeviceInfoClearAlias(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceDefPtr device ATTRIBUTE_UNUSED, virDomainDeviceInfoPtr info, void *opaque ATTRIBUTE_UNUSED) { @@ -1837,6 +1838,7 @@ static int virDomainDeviceInfoClearAlias(virDomainDefPtr def ATTRIBUTE_UNUSED, } static int virDomainDeviceInfoClearPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceDefPtr device ATTRIBUTE_UNUSED, virDomainDeviceInfoPtr info, void *opaque ATTRIBUTE_UNUSED) { @@ -1852,55 +1854,101 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def, void *opaque) { int i; + virDomainDeviceDef device; - for (i = 0; i < def->ndisks ; i++) - if (cb(def, &def->disks[i]->info, opaque) < 0) + device.type = VIR_DOMAIN_DEVICE_DISK; + for (i = 0; i < def->ndisks ; i++) { + device.data.disk = def->disks[i]; + if (cb(def, &device, &def->disks[i]->info, opaque) < 0) return -1; - for (i = 0; i < def->nnets ; i++) - if (cb(def, &def->nets[i]->info, opaque) < 0) + } + device.type = VIR_DOMAIN_DEVICE_NET; + for (i = 0; i < def->nnets ; i++) { + device.data.net = def->nets[i]; + if (cb(def, &device, &def->nets[i]->info, opaque) < 0) return -1; - for (i = 0; i < def->nsounds ; i++) - if (cb(def, &def->sounds[i]->info, opaque) < 0) + } + device.type = VIR_DOMAIN_DEVICE_SOUND; + for (i = 0; i < def->nsounds ; i++) { + device.data.sound = def->sounds[i]; + if (cb(def, &device, &def->sounds[i]->info, opaque) < 0) return -1; - for (i = 0; i < def->nhostdevs ; i++) - if (cb(def, &def->hostdevs[i]->info, opaque) < 0) + } + device.type = VIR_DOMAIN_DEVICE_HOSTDEV; + for (i = 0; i < def->nhostdevs ; i++) { + device.data.hostdev = def->hostdevs[i]; + if (cb(def, &device, &def->hostdevs[i]->info, opaque) < 0) return -1; - for (i = 0; i < def->nvideos ; i++) - if (cb(def, &def->videos[i]->info, opaque) < 0) + } + device.type = VIR_DOMAIN_DEVICE_VIDEO; + for (i = 0; i < def->nvideos ; i++) { + device.data.video = def->videos[i]; + if (cb(def, &device, &def->videos[i]->info, opaque) < 0) return -1; - for (i = 0; i < def->ncontrollers ; i++) - if (cb(def, &def->controllers[i]->info, opaque) < 0) + } + device.type = VIR_DOMAIN_DEVICE_CONTROLLER; + for (i = 0; i < def->ncontrollers ; i++) { + device.data.controller = def->controllers[i]; + if (cb(def, &device, &def->controllers[i]->info, opaque) < 0) return -1; - for (i = 0; i < def->nsmartcards ; i++) - if (cb(def, &def->smartcards[i]->info, opaque) < 0) + } + device.type = VIR_DOMAIN_DEVICE_SMARTCARD; + for (i = 0; i < def->nsmartcards ; i++) { + device.data.smartcard = def->smartcards[i]; + if (cb(def, &device, &def->smartcards[i]->info, opaque) < 0) return -1; - for (i = 0; i < def->nserials ; i++) - if (cb(def, &def->serials[i]->info, opaque) < 0) + } + device.type = VIR_DOMAIN_DEVICE_CHR; + for (i = 0; i < def->nserials ; i++) { + device.data.chr = def->serials[i]; + if (cb(def, &device, &def->serials[i]->info, opaque) < 0) return -1; - for (i = 0; i < def->nparallels ; i++) - if (cb(def, &def->parallels[i]->info, opaque) < 0) + } + for (i = 0; i < def->nparallels ; i++) { + device.data.chr = def->parallels[i]; + if (cb(def, &device, &def->parallels[i]->info, opaque) < 0) return -1; - for (i = 0; i < def->nchannels ; i++) - if (cb(def, &def->channels[i]->info, opaque) < 0) + } + for (i = 0; i < def->nchannels ; i++) { + device.data.chr = def->channels[i]; + if (cb(def, &device, &def->channels[i]->info, opaque) < 0) return -1; - for (i = 0; i < def->nconsoles ; i++) - if (cb(def, &def->consoles[i]->info, opaque) < 0) + } + for (i = 0; i < def->nconsoles ; i++) { + device.data.chr = def->consoles[i]; + if (cb(def, &device, &def->consoles[i]->info, opaque) < 0) return -1; - for (i = 0; i < def->ninputs ; i++) - if (cb(def, &def->inputs[i]->info, opaque) < 0) + } + device.type = VIR_DOMAIN_DEVICE_INPUT; + for (i = 0; i < def->ninputs ; i++) { + device.data.input = def->inputs[i]; + if (cb(def, &device, &def->inputs[i]->info, opaque) < 0) return -1; - for (i = 0; i < def->nfss ; i++) - if (cb(def, &def->fss[i]->info, opaque) < 0) + } + device.type = VIR_DOMAIN_DEVICE_FS; + for (i = 0; i < def->nfss ; i++) { + device.data.fs = def->fss[i]; + if (cb(def, &device, &def->fss[i]->info, opaque) < 0) return -1; - if (def->watchdog) - if (cb(def, &def->watchdog->info, opaque) < 0) + } + if (def->watchdog) { + device.type = VIR_DOMAIN_DEVICE_WATCHDOG; + device.data.watchdog = def->watchdog; + if (cb(def, &device, &def->watchdog->info, opaque) < 0) return -1; - if (def->memballoon) - if (cb(def, &def->memballoon->info, opaque) < 0) + } + if (def->memballoon) { + device.type = VIR_DOMAIN_DEVICE_MEMBALLOON; + device.data.memballoon = def->memballoon; + if (cb(def, &device, &def->memballoon->info, opaque) < 0) return -1; - for (i = 0; i < def->nhubs ; i++) - if (cb(def, &def->hubs[i]->info, opaque) < 0) + } + device.type = VIR_DOMAIN_DEVICE_HUB; + for (i = 0; i < def->nhubs ; i++) { + device.data.hub = def->hubs[i]; + if (cb(def, &device, &def->hubs[i]->info, opaque) < 0) return -1; + } return 0; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 68b7cfd..e9f2391 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1785,7 +1785,8 @@ void virDomainDefClearPCIAddresses(virDomainDefPtr def); void virDomainDefClearDeviceAliases(virDomainDefPtr def); typedef int (*virDomainDeviceInfoCallback)(virDomainDefPtr def, - virDomainDeviceInfoPtr dev, + virDomainDeviceDefPtr dev, + virDomainDeviceInfoPtr info, void *opaque); int virDomainDeviceInfoIterate(virDomainDefPtr def, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 04e580f..de3054a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -721,6 +721,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virBitmapPtr qemuCaps) static int qemuSpaprVIOFindByReg(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceDefPtr device ATTRIBUTE_UNUSED, virDomainDeviceInfoPtr info, void *opaque) { virDomainDeviceInfoPtr target = opaque; @@ -835,6 +836,7 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev) static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceDefPtr device ATTRIBUTE_UNUSED, virDomainDeviceInfoPtr info, void *opaque) { diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 0ca3186..a7f0899 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1481,6 +1481,7 @@ static inline int qemuFindDisk(virDomainDefPtr def, const char *dst) } static int qemuComparePCIDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceDefPtr device ATTRIBUTE_UNUSED, virDomainDeviceInfoPtr info1, void *opaque) { -- 1.7.7.6

In order to allow for a virDomainHostdevDef that uses the virDomainDeviceInfo of a "higher level" device (such as a virDomainNetDef), this patch changes the virDomainDeviceInfo in the HostdevDef into a virDomainDeviceInfoPtr. Rather than adding checks all over the code to check for a null info, we just guarantee that it is always valid. The new function virDomainHostdevDefAlloc() allocates a virDomainDeviceInfo and plugs it in, and virDomainHostdevDefFree() makes sure it is freed. There were 4 places allocating virDomainHostdevDefs, all of them parsers of one sort or another, and those have all had their VIR_ALLOC(hostdev) changed to virDomainHostdevDefAlloc(). Other than that, and the new functions, all the rest of the changes are just mechanical removals of "&" or changing "." to "->". --- V2: also add a virDomainDeviceInfoClear() function. src/conf/domain_conf.c | 63 +++++++++++++++++++++++++++------- src/conf/domain_conf.h | 4 ++- src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 83 ++++++++++++++++++++------------------------- src/qemu/qemu_hotplug.c | 28 ++++++++-------- src/xenxs/xen_sxpr.c | 5 ++- src/xenxs/xen_xm.c | 8 +++-- 7 files changed, 114 insertions(+), 79 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 03f8564..eb66223 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -786,6 +786,15 @@ bool virDomainObjTaint(virDomainObjPtr obj, return true; } +static void +virDomainDeviceInfoFree(virDomainDeviceInfoPtr info) +{ + if (info) { + virDomainDeviceInfoClear(info); + VIR_FREE(info); + } +} + static void virDomainGraphicsAuthDefClear(virDomainGraphicsAuthDefPtr def) @@ -1294,12 +1303,42 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def) VIR_FREE(def); } +virDomainHostdevDefPtr virDomainHostdevDefAlloc(void) +{ + virDomainHostdevDefPtr def = NULL; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + if (VIR_ALLOC(def->info) < 0) { + virReportOOMError(); + VIR_FREE(def); + return NULL; + } + return def; +} + +void virDomainHostdevDefClear(virDomainHostdevDefPtr def) +{ + if (!def) + return; + + /* Free all resources in the hostdevdef. Currently the only + * such resource is the virDomainDeviceInfo. + */ + + virDomainDeviceInfoFree(def->info); +} + void virDomainHostdevDefFree(virDomainHostdevDefPtr def) { if (!def) return; - virDomainDeviceInfoClear(&def->info); + /* free all subordinate objects */ + virDomainHostdevDefClear(def); + VIR_FREE(def); } @@ -1877,7 +1916,7 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def, device.type = VIR_DOMAIN_DEVICE_HOSTDEV; for (i = 0; i < def->nhostdevs ; i++) { device.data.hostdev = def->hostdevs[i]; - if (cb(def, &device, &def->hostdevs[i]->info, opaque) < 0) + if (cb(def, &device, def->hostdevs[i]->info, opaque) < 0) return -1; } device.type = VIR_DOMAIN_DEVICE_VIDEO; @@ -2743,14 +2782,14 @@ virDomainHostdevSubsysPciDefParseXML(const xmlNodePtr node, char *devaddr = virXMLPropString(cur, "devaddr"); if (devaddr && virDomainParseLegacyDeviceAddress(devaddr, - &def->info.addr.pci) < 0) { + &def->info->addr.pci) < 0) { virDomainReportError(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; + def->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; } else if ((flags & VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES) && xmlStrEqual(cur->name, BAD_CAST "origstates")) { virDomainHostdevOrigStatesPtr states = &def->origstates; @@ -6379,10 +6418,8 @@ virDomainHostdevDefParseXML(const xmlNodePtr node, virDomainHostdevDefPtr def; char *mode, *type = NULL, *managed = NULL; - if (VIR_ALLOC(def) < 0) { - virReportOOMError(); + if (!(def = virDomainHostdevDefAlloc())) return NULL; - } mode = virXMLPropString(node, "mode"); if (mode) { @@ -6445,8 +6482,8 @@ virDomainHostdevDefParseXML(const xmlNodePtr node, cur = cur->next; } - if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - if (virDomainDeviceInfoParseXML(node, bootMap, &def->info, + if (def->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + if (virDomainDeviceInfoParseXML(node, bootMap, def->info, flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT | VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM) < 0) goto error; @@ -6455,8 +6492,8 @@ virDomainHostdevDefParseXML(const xmlNodePtr node, 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) { + if (def->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + def->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("PCI host devices must use 'pci' address type")); goto error; @@ -9015,7 +9052,7 @@ static bool virDomainHostdevDefCheckABIStability(virDomainHostdevDefPtr src, } } - if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) + if (!virDomainDeviceInfoCheckABIStability(src->info, dst->info)) goto cleanup; identical = true; @@ -11551,7 +11588,7 @@ virDomainHostdevDefFormat(virBufferPtr buf, virBufferAddLit(buf, " </source>\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, + if (virDomainDeviceInfoFormat(buf, def->info, flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT | VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM) < 0) return -1; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e9f2391..7815ee7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -380,7 +380,7 @@ struct _virDomainHostdevDef { } caps; } source; virDomainHostdevOrigStates origstates; - virDomainDeviceInfo info; /* Guest address */ + virDomainDeviceInfoPtr info; /* Guest address */ }; /* Two types of disk backends */ @@ -1773,6 +1773,8 @@ void virDomainSoundDefFree(virDomainSoundDefPtr def); void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def); void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def); void virDomainVideoDefFree(virDomainVideoDefPtr def); +virDomainHostdevDefPtr virDomainHostdevDefAlloc(void); +void virDomainHostdevDefClear(virDomainHostdevDefPtr def); void virDomainHostdevDefFree(virDomainHostdevDefPtr def); void virDomainHubDefFree(virDomainHubDefPtr def); void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b9baf9a..b2c0c71 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -351,6 +351,8 @@ virDomainGraphicsSpiceZlibCompressionTypeFromString; virDomainGraphicsSpiceZlibCompressionTypeToString; virDomainGraphicsTypeFromString; virDomainGraphicsTypeToString; +virDomainHostdevDefAlloc; +virDomainHostdevDefClear; virDomainHostdevDefFree; virDomainHostdevModeTypeToString; virDomainHostdevSubsysTypeToString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index de3054a..db02323 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -570,7 +570,7 @@ qemuAssignDeviceHostdevAlias(virDomainDefPtr def, virDomainHostdevDefPtr hostdev idx = 0; for (i = 0 ; i < def->nhostdevs ; i++) { int thisidx; - if ((thisidx = qemuDomainDeviceAliasIndex(&def->hostdevs[i]->info, "hostdev")) < 0) { + if ((thisidx = qemuDomainDeviceAliasIndex(def->hostdevs[i]->info, "hostdev")) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to determine device index for hostdev device")); return -1; @@ -580,7 +580,7 @@ qemuAssignDeviceHostdevAlias(virDomainDefPtr def, virDomainHostdevDefPtr hostdev } } - if (virAsprintf(&hostdev->info.alias, "hostdev%d", idx) < 0) { + if (virAsprintf(&hostdev->info->alias, "hostdev%d", idx) < 0) { virReportOOMError(); return -1; } @@ -1421,13 +1421,13 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs) /* Host PCI devices */ for (i = 0; i < def->nhostdevs ; i++) { - if (def->hostdevs[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + if (def->hostdevs[i]->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) continue; if (def->hostdevs[i]->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || def->hostdevs[i]->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) continue; - if (qemuDomainPCIAddressSetNextAddr(addrs, &def->hostdevs[i]->info) < 0) + if (qemuDomainPCIAddressSetNextAddr(addrs, def->hostdevs[i]->info) < 0) goto error; } @@ -2979,14 +2979,14 @@ qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev, const char *configfd, dev->source.subsys.u.pci.bus, dev->source.subsys.u.pci.slot, dev->source.subsys.u.pci.function); - virBufferAsprintf(&buf, ",id=%s", dev->info.alias); + virBufferAsprintf(&buf, ",id=%s", dev->info->alias); if (configfd && *configfd) virBufferAsprintf(&buf, ",configfd=%s", configfd); - if (dev->info.bootIndex) - virBufferAsprintf(&buf, ",bootindex=%d", dev->info.bootIndex); - if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCaps) < 0) + if (dev->info->bootIndex) + virBufferAsprintf(&buf, ",bootindex=%d", dev->info->bootIndex); + if (qemuBuildDeviceAddressStr(&buf, dev->info, qemuCaps) < 0) goto error; - if (qemuBuildRomStr(&buf, &dev->info, qemuCaps) < 0) + if (qemuBuildRomStr(&buf, dev->info, qemuCaps) < 0) goto error; if (virBufferError(&buf)) { @@ -3072,9 +3072,9 @@ qemuBuildUSBHostdevDevStr(virDomainHostdevDefPtr dev, virBufferAsprintf(&buf, "usb-host,hostbus=%d,hostaddr=%d,id=%s", dev->source.subsys.u.usb.bus, dev->source.subsys.u.usb.device, - dev->info.alias); + dev->info->alias); - if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCaps) < 0) + if (qemuBuildDeviceAddressStr(&buf, dev->info, qemuCaps) < 0) goto error; if (virBufferError(&buf)) { @@ -5701,7 +5701,7 @@ qemuBuildCommandLine(virConnectPtr conn, virDomainHostdevDefPtr hostdev = def->hostdevs[i]; char *devstr; - if (hostdev->info.bootIndex) { + if (hostdev->info->bootIndex) { if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -6653,43 +6653,37 @@ cleanup: static virDomainHostdevDefPtr qemuParseCommandLinePCI(const char *val) { - virDomainHostdevDefPtr def = NULL; int bus = 0, slot = 0, func = 0; const char *start; char *end; + virDomainHostdevDefPtr def = virDomainHostdevDefAlloc(); + + if (!def) + goto error; if (!STRPREFIX(val, "host=")) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("unknown PCI device syntax '%s'"), val); - VIR_FREE(def); - goto cleanup; + goto error; } start = val + strlen("host="); if (virStrToLong_i(start, &end, 16, &bus) < 0 || *end != ':') { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("cannot extract PCI device bus '%s'"), val); - VIR_FREE(def); - goto cleanup; + goto error; } start = end + 1; if (virStrToLong_i(start, &end, 16, &slot) < 0 || *end != '.') { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("cannot extract PCI device slot '%s'"), val); - VIR_FREE(def); - goto cleanup; + goto error; } start = end + 1; if (virStrToLong_i(start, NULL, 16, &func) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("cannot extract PCI device function '%s'"), val); - VIR_FREE(def); - goto cleanup; - } - - if (VIR_ALLOC(def) < 0) { - virReportOOMError(); - goto cleanup; + goto error; } def->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; @@ -6698,9 +6692,11 @@ qemuParseCommandLinePCI(const char *val) def->source.subsys.u.pci.bus = bus; def->source.subsys.u.pci.slot = slot; def->source.subsys.u.pci.function = func; - -cleanup: return def; + + error: + virDomainHostdevDefFree(def); + return NULL; } @@ -6710,16 +6706,18 @@ cleanup: static virDomainHostdevDefPtr qemuParseCommandLineUSB(const char *val) { - virDomainHostdevDefPtr def = NULL; + virDomainHostdevDefPtr def = virDomainHostdevDefAlloc(); int first = 0, second = 0; const char *start; char *end; + if (!def) + goto error; + if (!STRPREFIX(val, "host:")) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("unknown USB device syntax '%s'"), val); - VIR_FREE(def); - goto cleanup; + goto error; } start = val + strlen("host:"); @@ -6727,37 +6725,28 @@ qemuParseCommandLineUSB(const char *val) if (virStrToLong_i(start, &end, 16, &first) < 0 || *end != ':') { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("cannot extract USB device vendor '%s'"), val); - VIR_FREE(def); - goto cleanup; + goto error; } start = end + 1; if (virStrToLong_i(start, NULL, 16, &second) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("cannot extract USB device product '%s'"), val); - VIR_FREE(def); - goto cleanup; + goto error; } } else { if (virStrToLong_i(start, &end, 10, &first) < 0 || *end != '.') { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("cannot extract USB device bus '%s'"), val); - VIR_FREE(def); - goto cleanup; + goto error; } start = end + 1; if (virStrToLong_i(start, NULL, 10, &second) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("cannot extract USB device address '%s'"), val); - VIR_FREE(def); - goto cleanup; + goto error; } } - if (VIR_ALLOC(def) < 0) { - virReportOOMError(); - goto cleanup; - } - def->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; def->managed = 0; def->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB; @@ -6768,9 +6757,11 @@ qemuParseCommandLineUSB(const char *val) def->source.subsys.u.usb.vendor = first; def->source.subsys.u.usb.product = second; } - -cleanup: return def; + + error: + virDomainHostdevDefFree(def); + return NULL; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index a7f0899..e727cfe 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -918,14 +918,14 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuAssignDeviceHostdevAlias(vm->def, hostdev, -1) < 0) goto error; - if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &hostdev->info) < 0) + if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, hostdev->info) < 0) goto error; releaseaddr = true; if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_PCI_CONFIGFD)) { configfd = qemuOpenPCIConfig(hostdev); if (configfd >= 0) { if (virAsprintf(&configfd_name, "fd-%s", - hostdev->info.alias) < 0) { + hostdev->info->alias) < 0) { virReportOOMError(); goto error; } @@ -947,7 +947,7 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, configfd, configfd_name); qemuDomainObjExitMonitorWithDriver(driver, vm); } else { - virDomainDevicePCIAddress guestAddr = hostdev->info.addr.pci; + virDomainDevicePCIAddress guestAddr = hostdev->info->addr.pci; qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorAddPCIHostDevice(priv->mon, @@ -955,8 +955,8 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, &guestAddr); qemuDomainObjExitMonitorWithDriver(driver, vm); - hostdev->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; - memcpy(&hostdev->info.addr.pci, &guestAddr, sizeof(guestAddr)); + hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + memcpy(&hostdev->info->addr.pci, &guestAddr, sizeof(guestAddr)); } virDomainAuditHostdev(vm, hostdev, "attach", ret == 0); if (ret < 0) @@ -972,10 +972,10 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, error: if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - (hostdev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && + (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && releaseaddr && qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, - hostdev->info.addr.pci.slot) < 0) + hostdev->info->addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on host device"); qemuDomainReAttachHostdevDevices(driver, vm->def->name, &hostdev, 1); @@ -2018,14 +2018,14 @@ qemuDomainDetachHostPciDevice(struct qemud_driver *driver, return -1; } - if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { + if (qemuIsMultiFunctionDevice(vm->def, detach->info)) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("cannot hot unplug multifunction PCI device: %s"), dev->data.disk->dst); return -1; } - if (!virDomainDeviceAddressIsValid(&detach->info, + if (!virDomainDeviceAddressIsValid(detach->info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("device cannot be detached without a PCI address")); @@ -2034,9 +2034,9 @@ qemuDomainDetachHostPciDevice(struct qemud_driver *driver, qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { - ret = qemuMonitorDelDevice(priv->mon, detach->info.alias); + ret = qemuMonitorDelDevice(priv->mon, detach->info->alias); } else { - ret = qemuMonitorRemovePCIDevice(priv->mon, &detach->info.addr.pci); + ret = qemuMonitorRemovePCIDevice(priv->mon, &detach->info->addr.pci); } qemuDomainObjExitMonitorWithDriver(driver, vm); virDomainAuditHostdev(vm, detach, "detach", ret == 0); @@ -2062,7 +2062,7 @@ qemuDomainDetachHostPciDevice(struct qemud_driver *driver, if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, - detach->info.addr.pci.slot) < 0) + detach->info->addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on host device"); if (vm->def->nhostdevs > 1) { @@ -2131,7 +2131,7 @@ qemuDomainDetachHostUsbDevice(struct qemud_driver *driver, return -1; } - if (!detach->info.alias) { + if (!detach->info->alias) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("device cannot be detached without a device alias")); return -1; @@ -2144,7 +2144,7 @@ qemuDomainDetachHostUsbDevice(struct qemud_driver *driver, } qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorDelDevice(priv->mon, detach->info.alias); + ret = qemuMonitorDelDevice(priv->mon, detach->info->alias); qemuDomainObjExitMonitorWithDriver(driver, vm); virDomainAuditHostdev(vm, detach, "detach", ret == 0); if (ret < 0) diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index f8390ea..8994cbc 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1076,8 +1076,8 @@ xenParseSxprPCI(virDomainDefPtr def, goto error; } - if (VIR_ALLOC(dev) < 0) - goto no_memory; + if (!(dev = virDomainHostdevDefAlloc())) + goto error; dev->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; dev->managed = 0; @@ -1088,6 +1088,7 @@ xenParseSxprPCI(virDomainDefPtr def, dev->source.subsys.u.pci.function = funcID; if (VIR_REALLOC_N(def->hostdevs, def->nhostdevs+1) < 0) { + virDomainHostdevDefFree(dev); goto no_memory; } diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index e580a3e..5862168 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -815,8 +815,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, if (virStrToLong_i(func, NULL, 16, &funcID) < 0) goto skippci; - if (VIR_ALLOC(hostdev) < 0) - goto no_memory; + if (!(hostdev = virDomainHostdevDefAlloc())) + goto cleanup; hostdev->managed = 0; hostdev->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI; @@ -825,8 +825,10 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, hostdev->source.subsys.u.pci.slot = slotID; hostdev->source.subsys.u.pci.function = funcID; - if (VIR_REALLOC_N(def->hostdevs, def->nhostdevs+1) < 0) + if (VIR_REALLOC_N(def->hostdevs, def->nhostdevs+1) < 0) { + virDomainHostdevDefFree(hostdev); goto no_memory; + } def->hostdevs[def->nhostdevs++] = hostdev; hostdev = NULL; -- 1.7.7.6

In an upcoming patch, virDomainNetDef will acquire a virDomainHostdevDef, and the <interface> XML will take on some of the elements of a <hostdev>. To avoid duplicating the code for parsing and formatting the <source> element (which will be nearly identical in these two cases), this patch factors those parts out of the HostdevDef's parse and format functions, and puts them into separate helper functions that are now called by the HostdevDef parser/formatter, and will soon be called by the NetDef parser/formatter. One change in behavior - previously virDomainHostdevDefParseXML() had diverged from current common coding practice by logging an error and failing if it found any subelements of <hostdev> other than those it understood (standard libvirt practice is to ignore/discard unknown elements and attributes during parse). The new helper function ignores unknown elements, and thus so does the new virDomainHostdevDefParseXML. --- V2: Unchanged from V1. src/conf/domain_conf.c | 263 +++++++++++++++++++++++++++++------------------- 1 files changed, 160 insertions(+), 103 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index eb66223..6d7c148 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2810,6 +2810,90 @@ out: return ret; } +static int +virDomainHostdevPartsParse(xmlNodePtr node, + xmlXPathContextPtr ctxt, + const char *mode, + const char *type, + virDomainHostdevDefPtr def, + unsigned int flags) +{ + xmlNodePtr sourcenode; + char *managed = NULL; + int ret = -1; + + /* @mode is passed in separately from the caller, since an + * 'intelligent hostdev' has no place for 'mode' in the XML (it is + * always 'subsys'). + */ + if (mode) { + if ((def->mode=virDomainHostdevModeTypeFromString(mode)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown hostdev mode '%s'"), mode); + goto error; + } + } else { + def->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; + } + + /* @managed can be read from the xml document - it is always an + * attribute of the toplevel element, no matter what type of + * element that might be (pure hostdev, or higher level device + * (e.g. <interface>) with type='hostdev') + */ + if ((managed = virXMLPropString(node, "managed"))!= NULL) { + if (STREQ(managed, "yes")) + def->managed = 1; + } + + /* @type is passed in from the caller rather than read from the + * xml document, because it is specified in different places for + * different kinds of defs - it is an attribute of + * <source>/<address> for an intelligent hostdev (<interface>), + * but an attribute of the toplevel element for a standard + * <hostdev>. (the functions we're going to call expect address + * type to already be known). + */ + if (type) { + if ((def->source.subsys.type + = virDomainHostdevSubsysTypeFromString(type)) < 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("unknown host device source address type '%s'"), + type); + goto error; + } + } else { + virDomainReportError(VIR_ERR_XML_ERROR, + "%s", _("missing source address type")); + goto error; + } + + if (!(sourcenode = virXPathNode("./source", ctxt))) { + virDomainReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing <source> element in hostdev device")); + goto error; + } + switch (def->source.subsys.type) { + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: + if (virDomainHostdevSubsysPciDefParseXML(sourcenode, def, flags) < 0) + goto error; + break; + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: + if (virDomainHostdevSubsysUsbDefParseXML(sourcenode, def) < 0) + goto error; + break; + default: + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("address type='%s' not supported in hostdev interfaces"), + virDomainHostdevSubsysTypeToString(def->source.subsys.type)); + goto error; + } + ret = 0; +error: + VIR_FREE(managed); + return ret; +} + int virDomainDiskFindControllerModel(virDomainDefPtr def, virDomainDiskDefPtr disk, @@ -6411,76 +6495,23 @@ error: static virDomainHostdevDefPtr virDomainHostdevDefParseXML(const xmlNodePtr node, + xmlXPathContextPtr ctxt, virBitmapPtr bootMap, unsigned int flags) { - xmlNodePtr cur; virDomainHostdevDefPtr def; - char *mode, *type = NULL, *managed = NULL; - - if (!(def = virDomainHostdevDefAlloc())) - return NULL; + xmlNodePtr save = ctxt->node; + char *mode = virXMLPropString(node, "mode"); + char *type = virXMLPropString(node, "type"); - mode = virXMLPropString(node, "mode"); - if (mode) { - if ((def->mode=virDomainHostdevModeTypeFromString(mode)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown hostdev mode '%s'"), mode); - goto error; - } - } else { - def->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; - } + ctxt->node = node; - type = virXMLPropString(node, "type"); - if (type) { - if ((def->source.subsys.type = virDomainHostdevSubsysTypeFromString(type)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown host device type '%s'"), type); - goto error; - } - } else { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing type in hostdev")); + if (!(def = virDomainHostdevDefAlloc())) goto error; - } - managed = virXMLPropString(node, "managed"); - if (managed != NULL) { - if (STREQ(managed, "yes")) - def->managed = 1; - VIR_FREE(managed); - } - - cur = node->children; - while (cur != NULL) { - if (cur->type == XML_ELEMENT_NODE) { - if (xmlStrEqual(cur->name, BAD_CAST "source")) { - if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { - if (virDomainHostdevSubsysUsbDefParseXML(cur, def) < 0) - goto error; - } - if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { - if (virDomainHostdevSubsysPciDefParseXML(cur, def, flags) < 0) - goto error; - } - } else if (xmlStrEqual(cur->name, BAD_CAST "address")) { - /* address is parsed as part of virDomainDeviceInfoParseXML */ - } else if (xmlStrEqual(cur->name, BAD_CAST "alias")) { - /* alias is parsed as part of virDomainDeviceInfoParseXML */ - } else if (xmlStrEqual(cur->name, BAD_CAST "boot")) { - /* boot is parsed as part of virDomainDeviceInfoParseXML */ - } else if (xmlStrEqual(cur->name, BAD_CAST "rom")) { - /* rombar is parsed as part of virDomainDeviceInfoParseXML */ - } else { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown node %s"), cur->name); - } - } - cur = cur->next; - } + /* parse managed/mode/type, and the <source> element */ + if (virDomainHostdevPartsParse(node, ctxt, mode, type, def, flags) < 0) + goto error; if (def->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { if (virDomainDeviceInfoParseXML(node, bootMap, def->info, @@ -6505,6 +6536,7 @@ virDomainHostdevDefParseXML(const xmlNodePtr node, cleanup: VIR_FREE(type); VIR_FREE(mode); + ctxt->node = save; return def; error: @@ -6671,7 +6703,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, goto error; } else if (xmlStrEqual(node->name, BAD_CAST "hostdev")) { dev->type = VIR_DOMAIN_DEVICE_HOSTDEV; - if (!(dev->data.hostdev = virDomainHostdevDefParseXML(node, NULL, + if (!(dev->data.hostdev = virDomainHostdevDefParseXML(node, ctxt, NULL, flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "controller")) { @@ -8208,9 +8240,9 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, if (n && VIR_ALLOC_N(def->hostdevs, n) < 0) goto no_memory; for (i = 0 ; i < n ; i++) { - virDomainHostdevDefPtr hostdev = virDomainHostdevDefParseXML(nodes[i], - bootMap, - flags); + virDomainHostdevDefPtr hostdev; + + hostdev = virDomainHostdevDefParseXML(nodes[i], ctxt, bootMap, flags); if (!hostdev) goto error; @@ -10532,6 +10564,64 @@ virDomainFSDefFormat(virBufferPtr buf, } static int +virDomainHostdevSourceFormat(virBufferPtr buf, + virDomainHostdevDefPtr def, + unsigned int flags, + bool includeTypeInAddr) +{ + virBufferAddLit(buf, "<source>\n"); + switch (def->source.subsys.type) + { + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: + if (def->source.subsys.u.usb.vendor) { + virBufferAsprintf(buf, " <vendor id='0x%.4x'/>\n", + def->source.subsys.u.usb.vendor); + virBufferAsprintf(buf, " <product id='0x%.4x'/>\n", + def->source.subsys.u.usb.product); + } + if (def->source.subsys.u.usb.bus || + def->source.subsys.u.usb.device) { + virBufferAsprintf(buf, " <address %sbus='%d' device='%d'/>\n", + includeTypeInAddr ? "type='usb' " : "", + def->source.subsys.u.usb.bus, + def->source.subsys.u.usb.device); + } + break; + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: + virBufferAsprintf(buf, " <address %sdomain='0x%.4x' bus='0x%.2x' " + "slot='0x%.2x' function='0x%.1x'/>\n", + includeTypeInAddr ? "type='pci' " : "", + 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_PCI_ORIG_STATES) && + (def->origstates.states.pci.unbind_from_stub || + def->origstates.states.pci.remove_slot || + def->origstates.states.pci.reprobe)) { + virBufferAddLit(buf, " <origstates>\n"); + if (def->origstates.states.pci.unbind_from_stub) + virBufferAddLit(buf, " <unbind/>\n"); + if (def->origstates.states.pci.remove_slot) + virBufferAddLit(buf, " <removeslot/>\n"); + if (def->origstates.states.pci.reprobe) + virBufferAddLit(buf, " <reprobe/>\n"); + virBufferAddLit(buf, " </origstates>\n"); + } + break; + default: + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected hostdev type %d"), + def->source.subsys.type); + return -1; + } + + virBufferAddLit(buf, "</source>\n"); + return 0; +} + +static int virDomainActualNetDefFormat(virBufferPtr buf, virDomainActualNetDefPtr def) { @@ -11549,44 +11639,11 @@ virDomainHostdevDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " <hostdev mode='%s' type='%s' managed='%s'>\n", mode, type, def->managed ? "yes" : "no"); - virBufferAddLit(buf, " <source>\n"); - if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { - if (def->source.subsys.u.usb.vendor) { - virBufferAsprintf(buf, " <vendor id='0x%.4x'/>\n", - def->source.subsys.u.usb.vendor); - virBufferAsprintf(buf, " <product id='0x%.4x'/>\n", - def->source.subsys.u.usb.product); - } - if (def->source.subsys.u.usb.bus || - def->source.subsys.u.usb.device) - virBufferAsprintf(buf, " <address bus='%d' device='%d'/>\n", - def->source.subsys.u.usb.bus, - def->source.subsys.u.usb.device); - } else if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { - virBufferAsprintf(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_PCI_ORIG_STATES) && - (def->origstates.states.pci.unbind_from_stub || - def->origstates.states.pci.remove_slot || - def->origstates.states.pci.reprobe)) { - virBufferAddLit(buf, " <origstates>\n"); - if (def->origstates.states.pci.unbind_from_stub) - virBufferAddLit(buf, " <unbind/>\n"); - if (def->origstates.states.pci.remove_slot) - virBufferAddLit(buf, " <removeslot/>\n"); - if (def->origstates.states.pci.reprobe) - virBufferAddLit(buf, " <reprobe/>\n"); - virBufferAddLit(buf, " </origstates>\n"); - } - } - - virBufferAddLit(buf, " </source>\n"); + virBufferAdjustIndent(buf, 6); + if (virDomainHostdevSourceFormat(buf, def, flags, false) < 0) + return -1; + virBufferAdjustIndent(buf, -6); if (virDomainDeviceInfoFormat(buf, def->info, flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT -- 1.7.7.6

The parent can be any type of device. It defaults to type=none, and a NULL pointer. The intent is that if a hostdevdef is contained in the def for a higher level device (e.g. virDomainNetDef), hostdev->parent will point to the higher level device, and type will be set to that type of device. This way, during attach and detach of the device, parent can be checked, and appropriate callouts made to do higher level device initialization (e.g. setting MAC address). Also, although these hostdevs with parents will be added to a domain's hostdevs list, they will be treated slightly differently when traversing the list, e.g. virDomainHostdefDefFree for a hostdev that has a parent doesn't need to be called (and will be a NOP); it will simply be removed from the list (since the parent device object is in its own type-specific list, and will be freed from there). --- V2: unchanged from V1 src/conf/domain_conf.c | 12 ++++++++++-- src/conf/domain_conf.h | 1 + 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6d7c148..93fd8d7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1328,7 +1328,11 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) * such resource is the virDomainDeviceInfo. */ - virDomainDeviceInfoFree(def->info); + /* If there is a parent device object, it will handle freeing + * def->info. + */ + if (def->parent.type == VIR_DOMAIN_DEVICE_NONE) + virDomainDeviceInfoFree(def->info); } void virDomainHostdevDefFree(virDomainHostdevDefPtr def) @@ -1339,7 +1343,11 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def) /* free all subordinate objects */ virDomainHostdevDefClear(def); - VIR_FREE(def); + /* If there is a parent device object, it will handle freeing + * the memory. + */ + if (def->parent.type == VIR_DOMAIN_DEVICE_NONE) + VIR_FREE(def); } void virDomainHubDefFree(virDomainHubDefPtr def) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7815ee7..efb86bc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -356,6 +356,7 @@ enum virDomainHostdevSubsysType { /* basic device for direct passthrough */ struct _virDomainHostdevDef { + virDomainDeviceDef parent; /* higher level Def containing this */ int mode; /* enum virDomainHostdevMode */ unsigned int managed : 1; union { -- 1.7.7.6

To shorten some new code that accesses the many fields within the subsys struct of a hostdev, create a separate toplevel, typedefed virDomainHostdevSubsys struct so that we can define temporary pointers to the subsys part. --- New patch for V2. src/conf/domain_conf.h | 31 ++++++++++++++++++------------- 1 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index efb86bc..1a29fdb 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -354,25 +354,30 @@ enum virDomainHostdevSubsysType { VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST }; + +typedef struct _virDomainHostdevSubsys virDomainHostdevSubsys; +typedef virDomainHostdevSubsys *virDomainHostdevSubsysPtr; +struct _virDomainHostdevSubsys { + int type; /* enum virDomainHostdevBusType */ + union { + struct { + unsigned bus; + unsigned device; + + unsigned vendor; + unsigned product; + } usb; + virDomainDevicePCIAddress pci; /* host address */ + } u; +}; + /* basic device for direct passthrough */ struct _virDomainHostdevDef { virDomainDeviceDef parent; /* higher level Def containing this */ int mode; /* enum virDomainHostdevMode */ unsigned int managed : 1; union { - struct { - int type; /* enum virDomainHostdevBusType */ - union { - struct { - unsigned bus; - unsigned device; - - unsigned vendor; - unsigned product; - } usb; - virDomainDevicePCIAddress pci; /* host address */ - } u; - } subsys; + virDomainHostdevSubsys subsys; struct { /* TBD: struct capabilities see: * https://www.redhat.com/archives/libvir-list/2008-July/msg00429.html -- 1.7.7.6

Three new functions useful in other files: virDomainHostdevInsert: Add a new hostdev at the end of the array. This would more sensibly be called virDomainHostdevAppend, but the existing functions for other types of devices are called Insert. virDomainHostdevRemove: Eliminates one entry from the hostdevs array, but doesn't free it; patterned after the code at the end of the two qemuDomainDetachHostXXXDevice functions (and also other pre-existing virDomainXXXRemove functions for other device types). virDomainHostdevFind: This function is patterned from the search loops at the top of qemuDomainDetachHostPciDevice and qemuDomainDetachHostUsbDevice, and will be used to re-factor those (and other detach-related) functions. --- New patch for V2. src/conf/domain_conf.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 5 ++ src/libvirt_private.syms | 3 + 3 files changed, 102 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 93fd8d7..94ee634 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6769,6 +6769,100 @@ virDomainChrTargetTypeToString(int deviceType, } int +virDomainHostdevInsert(virDomainDefPtr def, virDomainHostdevDefPtr hostdev) +{ + if (VIR_REALLOC_N(def->hostdevs, def->nhostdevs + 1) < 0) + return -1; + def->hostdevs[def->nhostdevs++] = hostdev; + return 0; +} + +void +virDomainHostdevRemove(virDomainDefPtr def, size_t i) +{ + if (def->nhostdevs > 1) { + memmove(def->hostdevs + i, + def->hostdevs + i + 1, + sizeof(*def->hostdevs) * + (def->nhostdevs - (i + 1))); + def->nhostdevs--; + if (VIR_REALLOC_N(def->hostdevs, def->nhostdevs) < 0) { + /* ignore, harmless */ + } + } else { + VIR_FREE(def->hostdevs); + def->nhostdevs = 0; + } +} + +/* Find an entry in hostdevs that matches the source spec in + * @match. return pointer to the entry in @found (if found is + * non-NULL). Returns index (within hostdevs) of matched entry, or -1 + * if no match was found. + */ +int +virDomainHostdevFind(virDomainDefPtr def, + virDomainHostdevDefPtr match, + virDomainHostdevDefPtr *found) +{ + virDomainHostdevDefPtr local_found; + virDomainHostdevSubsysPtr m_subsys = &match->source.subsys; + int i; + + if (!found) + found = &local_found; + *found = NULL; + + /* There is no code that uses _MODE_CAPABILITIES, and nothing to + * compare if it did, so don't allow it. + */ + if (match->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + return -1; + + for (i = 0 ; i < def->nhostdevs ; i++) { + virDomainHostdevDefPtr compare = def->hostdevs[i]; + virDomainHostdevSubsysPtr c_subsys = &compare->source.subsys; + + if ((compare->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) || + (c_subsys->type != m_subsys->type)) { + continue; + } + + switch (m_subsys->type) + { + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: + if ((c_subsys->u.pci.domain == m_subsys->u.pci.domain) && + (c_subsys->u.pci.bus == m_subsys->u.pci.bus) && + (c_subsys->u.pci.slot == m_subsys->u.pci.slot) && + (c_subsys->u.pci.function == m_subsys->u.pci.function)) { + *found = compare; + } + break; + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: + if (m_subsys->u.usb.bus && m_subsys->u.usb.device) { + /* specified by bus location on host */ + if ((c_subsys->u.usb.bus == m_subsys->u.usb.bus) && + (c_subsys->u.usb.device == m_subsys->u.usb.device)) { + *found = compare; + } + } else { + /* specified by product & vendor id */ + if ((c_subsys->u.usb.product == m_subsys->u.usb.product) && + (c_subsys->u.usb.vendor == m_subsys->u.usb.vendor)) { + *found = compare; + } + } + break; + default: + break; + } + if (*found) + break; + } + return *found ? i : -1; +} + +int virDomainDiskIndexByName(virDomainDefPtr def, const char *name, bool allow_ambiguous) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1a29fdb..343e48d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1895,6 +1895,11 @@ int virDomainNetIndexByMac(virDomainDefPtr def, const unsigned char *mac); int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net); int virDomainNetRemoveByMac(virDomainDefPtr def, const unsigned char *mac); +int virDomainHostdevInsert(virDomainDefPtr def, virDomainHostdevDefPtr hostdev); +void virDomainHostdevRemove(virDomainDefPtr def, size_t i); +int virDomainHostdevFind(virDomainDefPtr def, virDomainHostdevDefPtr match, + virDomainHostdevDefPtr *found); + int virDomainGraphicsListenGetType(virDomainGraphicsDefPtr def, size_t ii) ATTRIBUTE_NONNULL(1); int virDomainGraphicsListenSetType(virDomainGraphicsDefPtr def, size_t ii, int val) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b2c0c71..921bed0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -354,7 +354,10 @@ virDomainGraphicsTypeToString; virDomainHostdevDefAlloc; virDomainHostdevDefClear; virDomainHostdevDefFree; +virDomainHostdevFind; +virDomainHostdevInsert; virDomainHostdevModeTypeToString; +virDomainHostdevRemove; virDomainHostdevSubsysTypeToString; virDomainHubTypeFromString; virDomainHubTypeToString; -- 1.7.7.6

Code movement only, no functional change. This is necessary to prevent a forward reference in an upcoming patch. --- New patch for V2. src/qemu/qemu_hotplug.c | 289 ++++++++++++++++++++++++----------------------- 1 files changed, 145 insertions(+), 144 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e727cfe..e9df537 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1833,150 +1833,6 @@ cleanup: return ret; } -int qemuDomainDetachNetDevice(struct qemud_driver *driver, - virDomainObjPtr vm, - virDomainDeviceDefPtr dev) -{ - int i, ret = -1; - virDomainNetDefPtr detach = NULL; - qemuDomainObjPrivatePtr priv = vm->privateData; - int vlan; - char *hostnet_name = NULL; - virNetDevVPortProfilePtr vport = NULL; - - for (i = 0 ; i < vm->def->nnets ; i++) { - virDomainNetDefPtr net = vm->def->nets[i]; - - if (!memcmp(net->mac, dev->data.net->mac, sizeof(net->mac))) { - detach = net; - break; - } - } - - if (!detach) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - _("network device %02x:%02x:%02x:%02x:%02x:%02x not found"), - dev->data.net->mac[0], dev->data.net->mac[1], - dev->data.net->mac[2], dev->data.net->mac[3], - dev->data.net->mac[4], dev->data.net->mac[5]); - goto cleanup; - } - - if (!virDomainDeviceAddressIsValid(&detach->info, - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("device cannot be detached without a PCI address")); - goto cleanup; - } - - if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - _("cannot hot unplug multifunction PCI device :%s"), - dev->data.disk->dst); - goto cleanup; - } - - if ((vlan = qemuDomainNetVLAN(detach)) < 0) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("unable to determine original VLAN")); - goto cleanup; - } - - if (virAsprintf(&hostnet_name, "host%s", detach->info.alias) < 0) { - virReportOOMError(); - goto cleanup; - } - - qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { - if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - virDomainAuditNet(vm, detach, NULL, "detach", false); - goto cleanup; - } - } else { - if (qemuMonitorRemovePCIDevice(priv->mon, - &detach->info.addr.pci) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - virDomainAuditNet(vm, detach, NULL, "detach", false); - goto cleanup; - } - } - - if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV) && - qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { - if (qemuMonitorRemoveNetdev(priv->mon, hostnet_name) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - virDomainAuditNet(vm, detach, NULL, "detach", false); - goto cleanup; - } - } else { - if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - virDomainAuditNet(vm, detach, NULL, "detach", false); - goto cleanup; - } - } - qemuDomainObjExitMonitorWithDriver(driver, vm); - - virDomainAuditNet(vm, detach, NULL, "detach", true); - - if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, - detach->info.addr.pci.slot) < 0) - VIR_WARN("Unable to release PCI address on NIC"); - - virDomainConfNWFilterTeardown(detach); - - if (virDomainNetGetActualType(detach) == VIR_DOMAIN_NET_TYPE_DIRECT) { - ignore_value(virNetDevMacVLanDeleteWithVPortProfile( - detach->ifname, detach->mac, - virDomainNetGetActualDirectDev(detach), - virDomainNetGetActualDirectMode(detach), - virDomainNetGetActualVirtPortProfile(detach), - driver->stateDir)); - VIR_FREE(detach->ifname); - } - - if ((driver->macFilter) && (detach->ifname != NULL)) { - if ((errno = networkDisallowMacOnPort(driver, - detach->ifname, - detach->mac))) { - virReportSystemError(errno, - _("failed to remove ebtables rule on '%s'"), - detach->ifname); - } - } - - vport = virDomainNetGetActualVirtPortProfile(detach); - if (vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) - ignore_value(virNetDevOpenvswitchRemovePort( - virDomainNetGetActualBridgeName(detach), - detach->ifname)); - - networkReleaseActualDevice(detach); - if (vm->def->nnets > 1) { - memmove(vm->def->nets + i, - vm->def->nets + i + 1, - sizeof(*vm->def->nets) * - (vm->def->nnets - (i + 1))); - vm->def->nnets--; - if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets) < 0) { - /* ignore, harmless */ - } - } else { - VIR_FREE(vm->def->nets); - vm->def->nnets = 0; - } - virDomainNetDefFree(detach); - - ret = 0; - -cleanup: - VIR_FREE(hostnet_name); - return ret; -} - static int qemuDomainDetachHostPciDevice(struct qemud_driver *driver, virDomainObjPtr vm, @@ -2222,6 +2078,151 @@ int qemuDomainDetachHostDevice(struct qemud_driver *driver, } int +qemuDomainDetachNetDevice(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev) +{ + int i, ret = -1; + virDomainNetDefPtr detach = NULL; + qemuDomainObjPrivatePtr priv = vm->privateData; + int vlan; + char *hostnet_name = NULL; + virNetDevVPortProfilePtr vport = NULL; + + for (i = 0 ; i < vm->def->nnets ; i++) { + virDomainNetDefPtr net = vm->def->nets[i]; + + if (!memcmp(net->mac, dev->data.net->mac, sizeof(net->mac))) { + detach = net; + break; + } + } + + if (!detach) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("network device %02x:%02x:%02x:%02x:%02x:%02x not found"), + dev->data.net->mac[0], dev->data.net->mac[1], + dev->data.net->mac[2], dev->data.net->mac[3], + dev->data.net->mac[4], dev->data.net->mac[5]); + goto cleanup; + } + + if (!virDomainDeviceAddressIsValid(&detach->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("device cannot be detached without a PCI address")); + goto cleanup; + } + + if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot hot unplug multifunction PCI device :%s"), + dev->data.disk->dst); + goto cleanup; + } + + if ((vlan = qemuDomainNetVLAN(detach)) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("unable to determine original VLAN")); + goto cleanup; + } + + if (virAsprintf(&hostnet_name, "host%s", detach->info.alias) < 0) { + virReportOOMError(); + goto cleanup; + } + + qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { + if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { + qemuDomainObjExitMonitorWithDriver(driver, vm); + virDomainAuditNet(vm, detach, NULL, "detach", false); + goto cleanup; + } + } else { + if (qemuMonitorRemovePCIDevice(priv->mon, + &detach->info.addr.pci) < 0) { + qemuDomainObjExitMonitorWithDriver(driver, vm); + virDomainAuditNet(vm, detach, NULL, "detach", false); + goto cleanup; + } + } + + if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV) && + qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { + if (qemuMonitorRemoveNetdev(priv->mon, hostnet_name) < 0) { + qemuDomainObjExitMonitorWithDriver(driver, vm); + virDomainAuditNet(vm, detach, NULL, "detach", false); + goto cleanup; + } + } else { + if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0) { + qemuDomainObjExitMonitorWithDriver(driver, vm); + virDomainAuditNet(vm, detach, NULL, "detach", false); + goto cleanup; + } + } + qemuDomainObjExitMonitorWithDriver(driver, vm); + + virDomainAuditNet(vm, detach, NULL, "detach", true); + + if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + detach->info.addr.pci.slot) < 0) + VIR_WARN("Unable to release PCI address on NIC"); + + virDomainConfNWFilterTeardown(detach); + + if (virDomainNetGetActualType(detach) == VIR_DOMAIN_NET_TYPE_DIRECT) { + ignore_value(virNetDevMacVLanDeleteWithVPortProfile( + detach->ifname, detach->mac, + virDomainNetGetActualDirectDev(detach), + virDomainNetGetActualDirectMode(detach), + virDomainNetGetActualVirtPortProfile(detach), + driver->stateDir)); + VIR_FREE(detach->ifname); + } + + if ((driver->macFilter) && (detach->ifname != NULL)) { + if ((errno = networkDisallowMacOnPort(driver, + detach->ifname, + detach->mac))) { + virReportSystemError(errno, + _("failed to remove ebtables rule on '%s'"), + detach->ifname); + } + } + + vport = virDomainNetGetActualVirtPortProfile(detach); + if (vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) + ignore_value(virNetDevOpenvswitchRemovePort( + virDomainNetGetActualBridgeName(detach), + detach->ifname)); + + networkReleaseActualDevice(detach); + if (vm->def->nnets > 1) { + memmove(vm->def->nets + i, + vm->def->nets + i + 1, + sizeof(*vm->def->nets) * + (vm->def->nnets - (i + 1))); + vm->def->nnets--; + if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets) < 0) { + /* ignore, harmless */ + } + } else { + VIR_FREE(vm->def->nets); + vm->def->nnets = 0; + } + virDomainNetDefFree(detach); + + ret = 0; + +cleanup: + VIR_FREE(hostnet_name); + return ret; +} + +int qemuDomainChangeGraphicsPasswords(struct qemud_driver *driver, virDomainObjPtr vm, int type, -- 1.7.7.6

This refactoring is necessary to support hotplug detach of type=hostdev network devices, but needs to be in a separate patch to make potential debugging of regressions more practical. Rather than the lowest level functions searching for a matching device, the search is now done in the toplevel function, and an intermediate-level function (qemuDomainDetachThisHostDevice()), which expects that the device's entry is already found, is called (this intermediate function will be called by qemuDomainDetachNetDevice() in order to support detach of type=hostdev net devices) This patch should result in 0 differences in functionality. --- New patch for V2. src/qemu/qemu_hotplug.c | 228 +++++++++++++++++++---------------------------- 1 files changed, 93 insertions(+), 135 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e9df537..cb41388 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1,7 +1,7 @@ /* * qemu_hotplug.h: QEMU device hotplug management * - * Copyright (C) 2006-2011 Red Hat, Inc. + * Copyright (C) 2006-2012 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1836,48 +1836,20 @@ cleanup: static int qemuDomainDetachHostPciDevice(struct qemud_driver *driver, virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - virDomainHostdevDefPtr *detach_ret) + virDomainHostdevDefPtr detach, + int idx) { - virDomainHostdevDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; - int i, ret; + virDomainHostdevSubsysPtr subsys = &detach->source.subsys; + int ret; pciDevice *pci; pciDevice *activePci; - for (i = 0 ; i < vm->def->nhostdevs ; i++) { - if (vm->def->hostdevs[i]->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || - vm->def->hostdevs[i]->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) - continue; - - unsigned domain = vm->def->hostdevs[i]->source.subsys.u.pci.domain; - unsigned bus = vm->def->hostdevs[i]->source.subsys.u.pci.bus; - unsigned slot = vm->def->hostdevs[i]->source.subsys.u.pci.slot; - unsigned function = vm->def->hostdevs[i]->source.subsys.u.pci.function; - - if (dev->data.hostdev->source.subsys.u.pci.domain == domain && - dev->data.hostdev->source.subsys.u.pci.bus == bus && - dev->data.hostdev->source.subsys.u.pci.slot == slot && - dev->data.hostdev->source.subsys.u.pci.function == function) { - detach = vm->def->hostdevs[i]; - break; - } - } - - if (!detach) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - _("host pci device %.4x:%.2x:%.2x.%.1x not found"), - dev->data.hostdev->source.subsys.u.pci.domain, - dev->data.hostdev->source.subsys.u.pci.bus, - dev->data.hostdev->source.subsys.u.pci.slot, - dev->data.hostdev->source.subsys.u.pci.function); - return -1; - } - if (qemuIsMultiFunctionDevice(vm->def, detach->info)) { qemuReportError(VIR_ERR_OPERATION_FAILED, - _("cannot hot unplug multifunction PCI device: %s"), - dev->data.disk->dst); + _("cannot hot unplug multifunction PCI device: %.4x:%.2x:%.2x.%.1x"), + subsys->u.pci.domain, subsys->u.pci.bus, + subsys->u.pci.slot, subsys->u.pci.function); return -1; } @@ -1899,10 +1871,8 @@ qemuDomainDetachHostPciDevice(struct qemud_driver *driver, if (ret < 0) return -1; - pci = pciGetDevice(detach->source.subsys.u.pci.domain, - detach->source.subsys.u.pci.bus, - detach->source.subsys.u.pci.slot, - detach->source.subsys.u.pci.function); + pci = pciGetDevice(subsys->u.pci.domain, subsys->u.pci.bus, + subsys->u.pci.slot, subsys->u.pci.function); if (pci) { activePci = pciDeviceListSteal(driver->activePciHostdevs, pci); if (pciResetDevice(activePci, driver->activePciHostdevs, @@ -1921,71 +1891,20 @@ qemuDomainDetachHostPciDevice(struct qemud_driver *driver, detach->info->addr.pci.slot) < 0) VIR_WARN("Unable to release PCI address on host device"); - if (vm->def->nhostdevs > 1) { - memmove(vm->def->hostdevs + i, - vm->def->hostdevs + i + 1, - sizeof(*vm->def->hostdevs) * - (vm->def->nhostdevs - (i + 1))); - vm->def->nhostdevs--; - if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs) < 0) { - /* ignore, harmless */ - } - } else { - VIR_FREE(vm->def->hostdevs); - vm->def->nhostdevs = 0; - } - if (detach_ret) - *detach_ret = detach; - else - virDomainHostdevDefFree(detach); - + virDomainHostdevRemove(vm->def, idx); return ret; } static int qemuDomainDetachHostUsbDevice(struct qemud_driver *driver, virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - virDomainHostdevDefPtr *detach_ret) + virDomainHostdevDefPtr detach, + int idx) { - virDomainHostdevDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainHostdevSubsysPtr subsys = &detach->source.subsys; usbDevice *usb; - int i, ret; - - for (i = 0 ; i < vm->def->nhostdevs ; i++) { - if (vm->def->hostdevs[i]->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || - vm->def->hostdevs[i]->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) - continue; - - unsigned bus = vm->def->hostdevs[i]->source.subsys.u.usb.bus; - unsigned device = vm->def->hostdevs[i]->source.subsys.u.usb.device; - unsigned product = vm->def->hostdevs[i]->source.subsys.u.usb.product; - unsigned vendor = vm->def->hostdevs[i]->source.subsys.u.usb.vendor; - - if (dev->data.hostdev->source.subsys.u.usb.bus && - dev->data.hostdev->source.subsys.u.usb.device) { - if (dev->data.hostdev->source.subsys.u.usb.bus == bus && - dev->data.hostdev->source.subsys.u.usb.device == device) { - detach = vm->def->hostdevs[i]; - break; - } - } else { - if (dev->data.hostdev->source.subsys.u.usb.product == product && - dev->data.hostdev->source.subsys.u.usb.vendor == vendor) { - detach = vm->def->hostdevs[i]; - break; - } - } - } - - if (!detach) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - _("host usb device %03d.%03d not found"), - dev->data.hostdev->source.subsys.u.usb.bus, - dev->data.hostdev->source.subsys.u.usb.device); - return -1; - } + int ret; if (!detach->info->alias) { qemuReportError(VIR_ERR_OPERATION_FAILED, @@ -2006,77 +1925,116 @@ qemuDomainDetachHostUsbDevice(struct qemud_driver *driver, if (ret < 0) return -1; - usb = usbGetDevice(detach->source.subsys.u.usb.bus, - detach->source.subsys.u.usb.device); + usb = usbGetDevice(subsys->u.usb.bus, subsys->u.usb.device); if (usb) { usbDeviceListDel(driver->activeUsbHostdevs, usb); usbFreeDevice(usb); } else { VIR_WARN("Unable to find device %03d.%03d in list of used USB devices", - detach->source.subsys.u.usb.bus, - detach->source.subsys.u.usb.device); - } - - if (vm->def->nhostdevs > 1) { - memmove(vm->def->hostdevs + i, - vm->def->hostdevs + i + 1, - sizeof(*vm->def->hostdevs) * - (vm->def->nhostdevs - (i + 1))); - vm->def->nhostdevs--; - if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs) < 0) { - /* ignore, harmless */ - } - } else { - VIR_FREE(vm->def->hostdevs); - vm->def->nhostdevs = 0; + subsys->u.usb.bus, subsys->u.usb.device); } - if (detach_ret) - *detach_ret = detach; - else - virDomainHostdevDefFree(detach); + virDomainHostdevRemove(vm->def, idx); return ret; } -int qemuDomainDetachHostDevice(struct qemud_driver *driver, - virDomainObjPtr vm, - virDomainDeviceDefPtr dev) +static +int qemuDomainDetachThisHostDevice(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainHostdevDefPtr detach, + int idx) { - virDomainHostdevDefPtr hostdev = dev->data.hostdev; - virDomainHostdevDefPtr detach = NULL; - int ret; + int ret = -1; - if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) { - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("hostdev mode '%s' not supported"), - virDomainHostdevModeTypeToString(hostdev->mode)); - return -1; + if (idx < 0) { + /* caller didn't know index of hostdev in hostdevs list, so we + * need to find it. + */ + for (idx = 0; idx < vm->def->nhostdevs; idx++) { + if (vm->def->hostdevs[idx] == detach) + break; + } + if (idx >= vm->def->nhostdevs) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("device not found in hostdevs list (%d entries)"), + vm->def->nhostdevs); + return ret; + } } - switch (hostdev->source.subsys.type) { + switch (detach->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: - ret = qemuDomainDetachHostPciDevice(driver, vm, dev, &detach); + ret = qemuDomainDetachHostPciDevice(driver, vm, detach, idx); break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: - ret = qemuDomainDetachHostUsbDevice(driver, vm, dev, &detach); + ret = qemuDomainDetachHostUsbDevice(driver, vm, detach, idx); break; default: qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("hostdev subsys type '%s' not supported"), - virDomainHostdevSubsysTypeToString(hostdev->source.subsys.type)); + virDomainHostdevSubsysTypeToString(detach->source.subsys.type)); return -1; } if (ret == 0 && virSecurityManagerRestoreHostdevLabel(driver->securityManager, - vm->def, detach) < 0) + vm->def, detach) < 0) { VIR_WARN("Failed to restore host device labelling"); + } virDomainHostdevDefFree(detach); - return ret; } +/* search for a hostdev matching dev and detach it */ +int qemuDomainDetachHostDevice(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev) +{ + virDomainHostdevDefPtr hostdev = dev->data.hostdev; + virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; + virDomainHostdevDefPtr detach = NULL; + int idx; + + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("hostdev mode '%s' not supported"), + virDomainHostdevModeTypeToString(hostdev->mode)); + return -1; + } + + idx = virDomainHostdevFind(vm->def, hostdev, &detach); + + if (idx < 0) { + switch(subsys->type) { + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("host pci device %.4x:%.2x:%.2x.%.1x not found"), + subsys->u.pci.domain, subsys->u.pci.bus, + subsys->u.pci.slot, subsys->u.pci.function); + break; + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: + if (subsys->u.usb.bus && subsys->u.usb.device) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("host usb device %03d.%03d not found"), + subsys->u.usb.bus, subsys->u.usb.device); + } else { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("host usb device vendor=0x%.4x product=0x%.4x not found"), + subsys->u.usb.vendor, subsys->u.usb.product); + } + break; + default: + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected hostdev type %d"), subsys->type); + break; + } + return -1; + } + + return qemuDomainDetachThisHostDevice(driver, vm, detach, idx); +} + int qemuDomainDetachNetDevice(struct qemud_driver *driver, virDomainObjPtr vm, -- 1.7.7.6

This is the new interface type that sets up a PCI/USB network device to be assigned to the guest with PCI/USB passthrough after initializing some network device-specific things from the config (e.g. MAC address, virtualport profile parameters). Here is an example of the syntax: <interface type='hostdev' managed='yes'> <source> <address type='pci' domain='0' bus='0' slot='4' function='0'/> </source> <mac address='00:11:22:33:44:55'/> <address type='pci' domain='0' bus='0' slot='7' function='0'/> </interface> This would assign the PCI card from bus 0 slot 4 function 0 on the host, to bus 0 slot 7 function 0 on the guest, but would first set the MAC address of the card to 00:11:22:33:44:55. Although it's not expected to be used very much, usb network hostdevs are also supported for completeness. <source> syntax is identical to that for plain <hostdev> devices, except that the <address> element should have "type='usb'" added if it's specified: <interface type='hostdev'> <source> <address type='usb' bus='0' device='4'/> </source> <mac address='00:11:22:33:44:55'/> </interface> If the vendor/product form of usb specification is used, type='usb' is implied: <interface type='hostdev'> <source> <vendor id='0x0012'/> <product id='0x24dd'/> </source> <mac address='00:11:22:33:44:55'/> </interface> --- V2: address Eric's concerns from V1 - check for OOM after strdup - put in a NOP virDomainHostdevDefClear() rather than just commenting "there is nothing in the HostdevDef that needs to be freed" - eliminate inconsistent {} usage. docs/formatdomain.html.in | 41 +++++ docs/schemas/domaincommon.rng | 50 +++++++ src/conf/domain_conf.c | 154 ++++++++++++++++++-- src/conf/domain_conf.h | 10 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 1 + src/uml/uml_conf.c | 5 + src/xenxs/xen_sxpr.c | 1 + .../qemuxml2argvdata/qemuxml2argv-net-hostdev.xml | 48 ++++++ tests/qemuxml2xmltest.c | 1 + 10 files changed, 297 insertions(+), 15 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6fcca94..06de0ca 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2303,6 +2303,47 @@ ... </pre> + + <h5><a name="elementsNICSHostdev">PCI/USB Passthrough</a></h5> + + <p> + A PCI or USB network device (specified by the &lt;source&gt; + element) is directly assigned to the guest using generic device + passthrough, after first optionally setting the device's MAC + address to the configured value, and associating the device with + a VEPA or 802.1Qgh capable switch using an optionally specified + %lt;virtualport%gt; element (see the examples of virtualport + given above for type='direct' network devices). + <span class="since">Since 0.9.11</span> + </p> + + <p> + Note that this "intelligent passthrough" of network devices is + very similar to the functionality of a standard &lt;hostdev&gt; + device, the difference being that this method allows specifying + a MAC address and &lt;virtualport&gt; for the passed-through + device. If these capabilities are not required, of if you are + using a version of libvirt older than 0.9.11, you should use + standard &lt;hostdev&gt; to assign the device to the guest + instead of &lt;interface type='hostdev'/&gt;. + </p> + +<pre> + ... + &lt;devices&gt; + &lt;interface type='hostdev'&gt; + &lt;source&gt; + &lt;address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/&gt; + &lt;/source&gt; + &lt;mac address='52:54:00:6d:90:02'&gt; + &lt;virtualport type='802.1Qbh'&gt; + &lt;parameters profileid='finance'/&gt; + &lt;/virtualport&gt; + &lt;/interface&gt; + &lt;/devices&gt; + ...</pre> + + <h5><a name="elementsNICSMulticast">Multicast tunnel</a></h5> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3908733..a905457 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1416,6 +1416,56 @@ </optional> </interleave> </group> + <group> + <attribute name="type"> + <value>hostdev</value> + </attribute> + <optional> + <attribute name="managed"> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> + </optional> + <interleave> + <element name="source"> + <choice> + <group> + <ref name="usbproduct"/> + <optional> + <ref name="usbaddress"/> + </optional> + </group> + <element name="address"> + <choice> + <group> + <attribute name="type"> + <value>pci</value> + </attribute> + <ref name="pciaddress"/> + </group> + <group> + <attribute name="type"> + <value>usb</value> + </attribute> + <attribute name="bus"> + <ref name="usbAddr"/> + </attribute> + <attribute name="device"> + <ref name="usbPort"/> + </attribute> + </group> + </choice> + </element> + </choice> + </element> + <optional> + <ref name="virtualPortProfile"/> + </optional> + <ref name="interface-options"/> + </interleave> + </group> </choice> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 94ee634..70e9224 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -286,7 +286,8 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, "network", "bridge", "internal", - "direct") + "direct", + "hostdev") VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, "default", @@ -971,6 +972,10 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def) VIR_FREE(def->data.direct.linkdev); VIR_FREE(def->data.direct.virtPortProfile); break; + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + virDomainHostdevDefClear(&def->data.hostdev.def); + VIR_FREE(def->data.hostdev.virtPortProfile); + break; default: break; } @@ -1021,6 +1026,11 @@ void virDomainNetDefFree(virDomainNetDefPtr def) VIR_FREE(def->data.direct.virtPortProfile); break; + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + virDomainHostdevDefClear(&def->data.hostdev.def); + VIR_FREE(def->data.hostdev.virtPortProfile); + break; + case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_LAST: break; @@ -4115,7 +4125,9 @@ cleanup: static int virDomainActualNetDefParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, - virDomainActualNetDefPtr *def) + virDomainNetDefPtr parent, + virDomainActualNetDefPtr *def, + unsigned int flags) { virDomainActualNetDefPtr actual = NULL; int ret = -1; @@ -4123,6 +4135,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node, xmlNodePtr bandwidth_node = NULL; char *type = NULL; char *mode = NULL; + char *addrtype = NULL; if (VIR_ALLOC(actual) < 0) { virReportOOMError(); @@ -4144,6 +4157,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node, } if (actual->type != VIR_DOMAIN_NET_TYPE_BRIDGE && actual->type != VIR_DOMAIN_NET_TYPE_DIRECT && + actual->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && actual->type != VIR_DOMAIN_NET_TYPE_NETWORK) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported type '%s' in interface's <actual> element"), @@ -4179,6 +4193,34 @@ virDomainActualNetDefParseXML(xmlNodePtr node, (!(actual->data.direct.virtPortProfile = virNetDevVPortProfileParse(virtPortNode)))) goto error; + } else if (actual->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + xmlNodePtr virtPortNode = virXPathNode("./virtualport", ctxt); + virDomainHostdevDefPtr hostdev = &actual->data.hostdev.def; + + hostdev->parent.type = VIR_DOMAIN_DEVICE_NET; + hostdev->parent.data.net = parent; + hostdev->info = &parent->info; + /* The helper function expects type to already be found and + * passed in as a string, since it is in a different place in + * NetDef vs HostdevDef. + */ + addrtype = virXPathString("string(./source/address/@type)", ctxt); + /* if not explicitly stated, source/vendor implies usb device */ + if ((!addrtype) && virXPathNode("./source/vendor", ctxt) && + ((addrtype = strdup("usb")) == NULL)) { + virReportOOMError(); + goto error; + } + if (virDomainHostdevPartsParse(node, ctxt, NULL, addrtype, + hostdev, flags) < 0) { + goto error; + } + + if (virtPortNode && + (!(actual->data.hostdev.virtPortProfile = + virNetDevVPortProfileParse(virtPortNode)))) { + goto error; + } } bandwidth_node = virXPathNode("./bandwidth", ctxt); @@ -4192,6 +4234,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node, error: VIR_FREE(type); VIR_FREE(mode); + VIR_FREE(addrtype); virDomainActualNetDefFree(actual); ctxt->node = save_ctxt; @@ -4213,6 +4256,7 @@ virDomainNetDefParseXML(virCapsPtr caps, unsigned int flags) { virDomainNetDefPtr def; + virDomainHostdevDefPtr hostdev; xmlNodePtr cur; char *macaddr = NULL; char *type = NULL; @@ -4234,6 +4278,7 @@ virDomainNetDefParseXML(virCapsPtr caps, char *devaddr = NULL; char *mode = NULL; char *linkstate = NULL; + char *addrtype = NULL; virNWFilterHashTablePtr filterparams = NULL; virNetDevVPortProfilePtr virtPort = NULL; virDomainActualNetDefPtr actual = NULL; @@ -4286,7 +4331,8 @@ virDomainNetDefParseXML(virCapsPtr caps, } else if ((virtPort == NULL) && ((def->type == VIR_DOMAIN_NET_TYPE_DIRECT) || (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) || - (def->type == VIR_DOMAIN_NET_TYPE_BRIDGE)) && + (def->type == VIR_DOMAIN_NET_TYPE_BRIDGE) || + (def->type == VIR_DOMAIN_NET_TYPE_HOSTDEV)) && xmlStrEqual(cur->name, BAD_CAST "virtualport")) { if (!(virtPort = virNetDevVPortProfileParse(cur))) goto error; @@ -4338,8 +4384,10 @@ virDomainNetDefParseXML(virCapsPtr caps, (flags & VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET) && (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) && xmlStrEqual(cur->name, BAD_CAST "actual")) { - if (virDomainActualNetDefParseXML(cur, ctxt, &actual) < 0) + if (virDomainActualNetDefParseXML(cur, ctxt, def, + &actual, flags) < 0) { goto error; + } } else if (xmlStrEqual(cur->name, BAD_CAST "bandwidth")) { if (!(def->bandwidth = virNetDevBandwidthParse(cur))) goto error; @@ -4494,6 +4542,30 @@ virDomainNetDefParseXML(virCapsPtr caps, break; + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + hostdev = &def->data.hostdev.def; + hostdev->parent.type = VIR_DOMAIN_DEVICE_NET; + hostdev->parent.data.net = def; + hostdev->info = &def->info; + /* The helper function expects type to already be found and + * passed in as a string, since it is in a different place in + * NetDef vs HostdevDef. + */ + addrtype = virXPathString("string(./source/address/@type)", ctxt); + /* if not explicitly stated, source/vendor implies usb device */ + if ((!addrtype) && virXPathNode("./source/vendor", ctxt) && + ((addrtype = strdup("usb")) == NULL)) { + virReportOOMError(); + goto error; + } + if (virDomainHostdevPartsParse(node, ctxt, NULL, addrtype, + hostdev, flags) < 0) { + goto error; + } + def->data.hostdev.virtPortProfile = virtPort; + virtPort = NULL; + break; + case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_LAST: break; @@ -4629,6 +4701,7 @@ cleanup: VIR_FREE(devaddr); VIR_FREE(mode); VIR_FREE(linkstate); + VIR_FREE(addrtype); virNWFilterHashTableFree(filterparams); return def; @@ -10725,7 +10798,8 @@ virDomainHostdevSourceFormat(virBufferPtr buf, static int virDomainActualNetDefFormat(virBufferPtr buf, - virDomainActualNetDefPtr def) + virDomainActualNetDefPtr def, + unsigned int flags) { int ret = -1; const char *type; @@ -10741,14 +10815,12 @@ virDomainActualNetDefFormat(virBufferPtr buf, return ret; } - if (def->type != VIR_DOMAIN_NET_TYPE_BRIDGE && - def->type != VIR_DOMAIN_NET_TYPE_DIRECT && - def->type != VIR_DOMAIN_NET_TYPE_NETWORK) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected net type %s"), type); - goto error; + virBufferAsprintf(buf, " <actual type='%s'", type); + if ((def->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) && + def->data.hostdev.def.managed) { + virBufferAddLit(buf, " managed='yes'"); } - virBufferAsprintf(buf, " <actual type='%s'>\n", type); + virBufferAddLit(buf, ">\n"); switch (def->type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: @@ -10781,8 +10853,26 @@ virDomainActualNetDefFormat(virBufferPtr buf, goto error; virBufferAdjustIndent(buf, -8); break; - default: + + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + virBufferAdjustIndent(buf, 8); + if (virDomainHostdevSourceFormat(buf, &def->data.hostdev.def, + flags, true) < 0) { + return -1; + } + if (virNetDevVPortProfileFormat(def->data.hostdev.virtPortProfile, + buf) < 0) { + return -1; + } + virBufferAdjustIndent(buf, -8); break; + + case VIR_DOMAIN_NET_TYPE_NETWORK: + break; + default: + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected net type %s"), type); + goto error; } virBufferAdjustIndent(buf, 8); @@ -10810,7 +10900,12 @@ virDomainNetDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <interface type='%s'>\n", type); + virBufferAsprintf(buf, " <interface type='%s'", type); + if ((def->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) && + def->data.hostdev.def.managed) { + virBufferAddLit(buf, " managed='yes'"); + } + virBufferAddLit(buf, ">\n"); virBufferAsprintf(buf, " <mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n", @@ -10829,7 +10924,7 @@ virDomainNetDefFormat(virBufferPtr buf, return -1; virBufferAdjustIndent(buf, -6); if ((flags & VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET) && - (virDomainActualNetDefFormat(buf, def->data.network.actual) < 0)) + (virDomainActualNetDefFormat(buf, def->data.network.actual, flags) < 0)) return -1; break; @@ -10883,6 +10978,19 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferAdjustIndent(buf, -6); break; + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + virBufferAdjustIndent(buf, 6); + if (virDomainHostdevSourceFormat(buf, &def->data.hostdev.def, + flags, true) < 0) { + return -1; + } + if (virNetDevVPortProfileFormat(def->data.hostdev.virtPortProfile, + buf) < 0) { + return -1; + } + virBufferAdjustIndent(buf, -6); + break; + case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_LAST: break; @@ -14136,6 +14244,18 @@ virDomainNetGetActualDirectMode(virDomainNetDefPtr iface) return iface->data.network.actual->data.direct.mode; } +virDomainHostdevDefPtr +virDomainNetGetActualHostdev(virDomainNetDefPtr iface) +{ + if (iface->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) + return &iface->data.hostdev.def; + if ((iface->type == VIR_DOMAIN_NET_TYPE_NETWORK) && + (iface->data.network.actual->type == VIR_DOMAIN_NET_TYPE_HOSTDEV)) { + return &iface->data.network.actual->data.hostdev.def; + } + return NULL; +} + virNetDevVPortProfilePtr virDomainNetGetActualVirtPortProfile(virDomainNetDefPtr iface) { @@ -14144,6 +14264,8 @@ virDomainNetGetActualVirtPortProfile(virDomainNetDefPtr iface) return iface->data.direct.virtPortProfile; case VIR_DOMAIN_NET_TYPE_BRIDGE: return iface->data.bridge.virtPortProfile; + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + return iface->data.hostdev.virtPortProfile; case VIR_DOMAIN_NET_TYPE_NETWORK: if (!iface->data.network.actual) return NULL; @@ -14152,6 +14274,8 @@ virDomainNetGetActualVirtPortProfile(virDomainNetDefPtr iface) return iface->data.network.actual->data.direct.virtPortProfile; case VIR_DOMAIN_NET_TYPE_BRIDGE: return iface->data.network.actual->data.bridge.virtPortProfile; + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + return iface->data.network.actual->data.hostdev.virtPortProfile; default: return NULL; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 343e48d..6ab5f32 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -694,6 +694,7 @@ enum virDomainNetType { VIR_DOMAIN_NET_TYPE_BRIDGE, VIR_DOMAIN_NET_TYPE_INTERNAL, VIR_DOMAIN_NET_TYPE_DIRECT, + VIR_DOMAIN_NET_TYPE_HOSTDEV, VIR_DOMAIN_NET_TYPE_LAST, }; @@ -744,6 +745,10 @@ struct _virDomainActualNetDef { int mode; /* enum virMacvtapMode from util/macvtap.h */ virNetDevVPortProfilePtr virtPortProfile; } direct; + struct { + virDomainHostdevDef def; + virNetDevVPortProfilePtr virtPortProfile; + } hostdev; } data; virNetDevBandwidthPtr bandwidth; }; @@ -797,6 +802,10 @@ struct _virDomainNetDef { int mode; /* enum virMacvtapMode from util/macvtap.h */ virNetDevVPortProfilePtr virtPortProfile; } direct; + struct { + virDomainHostdevDef def; + virNetDevVPortProfilePtr virtPortProfile; + } hostdev; } data; struct { bool sndbuf_specified; @@ -1922,6 +1931,7 @@ int virDomainNetGetActualType(virDomainNetDefPtr iface); const char *virDomainNetGetActualBridgeName(virDomainNetDefPtr iface); const char *virDomainNetGetActualDirectDev(virDomainNetDefPtr iface); int virDomainNetGetActualDirectMode(virDomainNetDefPtr iface); +virDomainHostdevDefPtr virDomainNetGetActualHostdev(virDomainNetDefPtr iface); virNetDevVPortProfilePtr virDomainNetGetActualVirtPortProfile(virDomainNetDefPtr iface); virNetDevBandwidthPtr diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 921bed0..de02634 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -384,6 +384,7 @@ virDomainNetGetActualBandwidth; virDomainNetGetActualBridgeName; virDomainNetGetActualDirectDev; virDomainNetGetActualDirectMode; +virDomainNetGetActualHostdev; virDomainNetGetActualType; virDomainNetGetActualVirtPortProfile; virDomainNetIndexByMac; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index db02323..dfac389 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2722,6 +2722,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, case VIR_DOMAIN_NET_TYPE_BRIDGE: case VIR_DOMAIN_NET_TYPE_INTERNAL: case VIR_DOMAIN_NET_TYPE_DIRECT: + case VIR_DOMAIN_NET_TYPE_HOSTDEV: case VIR_DOMAIN_NET_TYPE_LAST: break; } diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index dbbbfda..0e281ff 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -257,6 +257,11 @@ umlBuildCommandLineNet(virConnectPtr conn, _("direct networking type not supported")); goto error; + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + umlReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("hostdev networking type not supported")); + goto error; + case VIR_DOMAIN_NET_TYPE_LAST: break; } diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index 8994cbc..e5df953 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1956,6 +1956,7 @@ xenFormatSxprNet(virConnectPtr conn, case VIR_DOMAIN_NET_TYPE_MCAST: case VIR_DOMAIN_NET_TYPE_INTERNAL: case VIR_DOMAIN_NET_TYPE_DIRECT: + case VIR_DOMAIN_NET_TYPE_HOSTDEV: case VIR_DOMAIN_NET_TYPE_LAST: break; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml new file mode 100644 index 0000000..504e4f6 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml @@ -0,0 +1,48 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219136</memory> + <currentMemory>219136</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <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='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <interface type='hostdev' managed='yes'> + <mac address='00:11:22:33:44:55'/> + <source> + <address type='pci' domain='0x0002' bus='0x03' slot='0x07' function='0x1'/> + </source> + <virtualport type='802.1Qbg'> + <parameters managerid='11' typeid='1193047' typeidversion='2' instanceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> + </virtualport> + </interface> + <interface type='hostdev'> + <mac address='11:11:22:33:44:55'/> + <source> + <address type='usb' bus='0' device='2'/> + </source> + </interface> + <interface type='hostdev'> + <mac address='22:11:22:33:44:55'/> + <source> + <vendor id='0x0012'/> + <product id='0x24dd'/> + </source> + </interface> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 4a2e016..03c75f8 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -171,6 +171,7 @@ mymain(void) DO_TEST("net-eth"); DO_TEST("net-eth-ifname"); DO_TEST("net-virtio-network-portgroup"); + DO_TEST("net-hostdev"); DO_TEST("sound"); DO_TEST("net-bandwidth"); -- 1.7.7.6

On 03/05/2012 03:45 PM, Michal Privoznik wrote: I put that case in when the test case only tested the parser and formatter, but then when I added the code to handle commandline generation in the next patch (along with extending the test case to test that as well) the new test began to fail. This was because it's impossible to test the vendor/product id format of address specification without having an actual device on the bus to compare against, and qemusml2argvtest can't do that, so I had to remove it. I actually thought that I'd squashed the removal back into the same test where I'd added it, which is what I'll do before pushing. (It's especially irrelevant since we've learned that allowing USB type='hostdev' interfaces is pointless (with the current hardware/drivers anyway) as the only reason to have it would be to set the mac address, but the guest os' netdevice driver will end up resetting the mac address we've set anyway :-()

On 28.02.2012 21:14, Laine Stump wrote: Maybe my brain is too small for this, but this looks useless to me. However, not a show stopper. ACK Michal

The code being replaced is exactly identical to the newly global function, right down to the comment. --- New patch in V2 src/qemu/qemu_hotplug.c | 14 +------------- 1 files changed, 1 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index cb41388..6119108 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2158,19 +2158,7 @@ qemuDomainDetachNetDevice(struct qemud_driver *driver, detach->ifname)); networkReleaseActualDevice(detach); - if (vm->def->nnets > 1) { - memmove(vm->def->nets + i, - vm->def->nets + i + 1, - sizeof(*vm->def->nets) * - (vm->def->nnets - (i + 1))); - vm->def->nnets--; - if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets) < 0) { - /* ignore, harmless */ - } - } else { - VIR_FREE(vm->def->nets); - vm->def->nnets = 0; - } + virDomainNetRemove(vm->def, i); virDomainNetDefFree(detach); ret = 0; -- 1.7.7.6

qemuDomainAttachNetDevice - re-ordered some things at start of function because networkAllocateActualDevice should always be run and a slot in def->nets always allocated, but host_net_add isn't needed if the actual type is hostdev. - if actual type is hostdev, defer to qemuDomainAttachHostDevice (which will reach up to the NetDef for things like MAC address when necessary). After return from qemuDomainAttachHostDevice, slip directly to cleanup, since the rest of the function is specific to emulated net devices. - put assignment of new NetDef into expanded def->nets down below cleanup: (but only on success) since it is also needed for emulated and hostdev net devices. qemuDomainDetachHostDevice - after locating the exact device to detach, check if it's a network device and, if so, use toplevel qemuDomainDetachNetDevice instead so that the def->nets list is properly updated, and 'actual device' properly returned to network pool if appropriate. Otherwise, for normal hostdevs, call the lower level qemuDomainDetachThisDevice. qemuDomainDetachNetDevice - This is where it gets a bit tricky. After locating the device on the def->nets list, if the network device type == hostdev, call the *lower level* qemuDomainDetachThisDevice (which will reach back up to the parent net device for MAC address / virtualport when appropriate, then clear the device out of def->hostdevs) before skipping past all the emulated net-device-specific code to cleanup:, where the network device is removed from def->nets, and the network device object is freed. In short, any time a hostdev-type network device is detached, we must go through the toplevel virDomaineDetachNetDevice function first and last, to make sure 1) the def->nnets list is properly managed, and 2) any device allocated with networkAllocateActualDevice is properly freed. At the same time, in the middle we need to go through the lower-level virDomainDetach*This*HostDevice to be sure that 1) the def->hostdevs list is properly managed, 2) the PCI device is properly detached from the guest and reattached to the host (if appropriate), and 3) any higher level setup/teardown is called at the appropriate time, by reaching back up to the NetDef config (part (3) will be covered in a separate patch). --- src/qemu/qemu_hotplug.c | 61 +++++++++++++++++++++++++++++++++------------- 1 files changed, 44 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6119108..50563c5 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -661,9 +661,9 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, bool iface_connected = false; int actualType; - if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_HOST_NET_ADD)) { - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("installed qemu version does not support host_net_add")); + /* preallocate new slot for device */ + if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets+1) < 0) { + virReportOOMError(); return -1; } @@ -672,9 +672,27 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, * to the one defined in the network definition. */ if (networkAllocateActualDevice(net) < 0) - goto cleanup; + return -1; actualType = virDomainNetGetActualType(net); + + if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + /* This is really a "smart hostdev", so it should be attached + * as a hostdev (the hostdev code will reach over into the + * netdev-specific code as appropriate), then also added to + * the nets list (see cleanup:) if successful. + */ + ret = qemuDomainAttachHostDevice(driver, vm, + virDomainNetGetActualHostdev(net)); + goto cleanup; + } + + if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_HOST_NET_ADD)) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("installed qemu version does not support host_net_add")); + goto cleanup; + } + if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || actualType == VIR_DOMAIN_NET_TYPE_NETWORK) { if ((tapfd = qemuNetworkIfaceConnect(vm->def, conn, driver, net, @@ -693,9 +711,6 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, goto cleanup; } - if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets+1) < 0) - goto no_memory; - if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_NET_NAME) || qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuAssignDeviceNetAlias(vm->def, net, -1) < 0) @@ -826,10 +841,10 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, ret = 0; - vm->def->nets[vm->def->nnets++] = net; - cleanup: - if (ret < 0) { + if (!ret) { + vm->def->nets[vm->def->nnets++] = net; + } else { if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && releaseaddr && @@ -2032,7 +2047,13 @@ int qemuDomainDetachHostDevice(struct qemud_driver *driver, return -1; } - return qemuDomainDetachThisHostDevice(driver, vm, detach, idx); + /* If this is a network hostdev, we need to use the higher-level detach + * function so that mac address / virtualport are reset + */ + if (detach->parent.type == VIR_DOMAIN_DEVICE_NET) + return qemuDomainDetachNetDevice(driver, vm, &detach->parent); + else + return qemuDomainDetachThisHostDevice(driver, vm, detach, idx); } int @@ -2056,6 +2077,13 @@ qemuDomainDetachNetDevice(struct qemud_driver *driver, } } + if (virDomainNetGetActualType(detach) == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + ret = qemuDomainDetachThisHostDevice(driver, vm, + virDomainNetGetActualHostdev(detach), + -1); + goto cleanup; + } + if (!detach) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("network device %02x:%02x:%02x:%02x:%02x:%02x not found"), @@ -2156,14 +2184,13 @@ qemuDomainDetachNetDevice(struct qemud_driver *driver, ignore_value(virNetDevOpenvswitchRemovePort( virDomainNetGetActualBridgeName(detach), detach->ifname)); - - networkReleaseActualDevice(detach); - virDomainNetRemove(vm->def, i); - virDomainNetDefFree(detach); - ret = 0; - cleanup: + if (!ret) { + networkReleaseActualDevice(detach); + virDomainNetRemove(vm->def, i); + virDomainNetDefFree(detach); + } VIR_FREE(hostnet_name); return ret; } -- 1.7.7.6

On 03/05/2012 03:46 PM, Michal Privoznik wrote: I wondered about this too, but am just copying existing behavior which, as you say, is relying on the fact that there's no leak, the item is just one longer (and that one extra in length will be used up the next time someone wants to grow the array). Thanks to you and to Eric for all the reviews!