[libvirt] [PATCH 0/3] Add controller support for the ESX driver

This series adds controller support for the ESX driver. Also adds required additions like the optional model attribute for the controller element to get rid of the disk driver name abuse and support for wide SCSI bus addresses. Matthias

The domain XML parsing code autogenerates disk address and controller elements when they are not explicitly specified. The code assumes a narrow SCSI bus (7 units per bus). ESX uses a wide SCSI bus (16 units per bus). This is a step towards controller support for the ESX driver. --- src/conf/capabilities.h | 3 +++ src/conf/domain_conf.c | 39 +++++++++++++++++++++++++++++---------- src/conf/domain_conf.h | 2 +- src/esx/esx_driver.c | 2 ++ src/qemu/qemu_conf.c | 9 +++++---- 5 files changed, 40 insertions(+), 15 deletions(-) diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index bdf44fa..9290c82 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -24,6 +24,8 @@ #ifndef __VIR_CAPABILITIES_H # define __VIR_CAPABILITIES_H +# include <stdbool.h> + # include "internal.h" # include "util.h" # include "buf.h" @@ -125,6 +127,7 @@ struct _virCaps { void (*privateDataFreeFunc)(void *); int (*privateDataXMLFormat)(virBufferPtr, void *); int (*privateDataXMLParse)(xmlXPathContextPtr, void *); + bool hasWideScsiBus; }; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 64b5cf3..cac4042 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1339,7 +1339,7 @@ virDomainParseLegacyDeviceAddress(char *devaddr, } int -virDomainDiskDefAssignAddress(virDomainDiskDefPtr def) +virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) { int idx = virDiskNameToIndex(def->dst); if (idx < 0) @@ -1347,12 +1347,30 @@ virDomainDiskDefAssignAddress(virDomainDiskDefPtr def) switch (def->bus) { case VIR_DOMAIN_DISK_BUS_SCSI: - /* For SCSI we define the default mapping to be 7 units - * per bus, 1 bus per controller, many controllers */ def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE; - def->info.addr.drive.controller = idx / 7; - def->info.addr.drive.bus = 0; - def->info.addr.drive.unit = idx % 7; + + if (caps->hasWideScsiBus) { + /* For a wide SCSI bus we define the default mapping to be + * 16 units per bus, 1 bus per controller, many controllers. + * Unit 7 is the SCSI controller itself. Therefore unit 7 + * cannot be assigned to disks and is skipped. + */ + def->info.addr.drive.controller = idx / 15; + def->info.addr.drive.bus = 0; + def->info.addr.drive.unit = idx % 15; + + /* Skip the SCSI controller at unit 7 */ + if (def->info.addr.drive.unit >= 7) { + ++def->info.addr.drive.unit; + } + } else { + /* For a narrow SCSI bus we define the default mapping to be + * 7 units per bus, 1 bus per controller, many controllers */ + def->info.addr.drive.controller = idx / 7; + def->info.addr.drive.bus = 0; + def->info.addr.drive.unit = idx % 7; + } + break; case VIR_DOMAIN_DISK_BUS_IDE: @@ -1385,7 +1403,8 @@ virDomainDiskDefAssignAddress(virDomainDiskDefPtr def) * @param node XML nodeset to parse for disk definition */ static virDomainDiskDefPtr -virDomainDiskDefParseXML(xmlNodePtr node, +virDomainDiskDefParseXML(virCapsPtr caps, + xmlNodePtr node, int flags) { virDomainDiskDefPtr def; xmlNodePtr cur; @@ -1615,7 +1634,7 @@ virDomainDiskDefParseXML(xmlNodePtr node, serial = NULL; if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE - && virDomainDiskDefAssignAddress(def) < 0) + && virDomainDiskDefAssignAddress(caps, def) < 0) goto error; cleanup: @@ -3687,7 +3706,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, if (xmlStrEqual(node->name, BAD_CAST "disk")) { dev->type = VIR_DOMAIN_DEVICE_DISK; - if (!(dev->data.disk = virDomainDiskDefParseXML(node, flags))) + if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "filesystem")) { dev->type = VIR_DOMAIN_DEVICE_FS; @@ -4237,7 +4256,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, if (n && VIR_ALLOC_N(def->disks, n) < 0) goto no_memory; for (i = 0 ; i < n ; i++) { - virDomainDiskDefPtr disk = virDomainDiskDefParseXML(nodes[i], + virDomainDiskDefPtr disk = virDomainDiskDefParseXML(caps, nodes[i], flags); if (!disk) goto error; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f83de83..701849f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -999,7 +999,7 @@ int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk); void virDomainDiskInsertPreAlloced(virDomainDefPtr def, virDomainDiskDefPtr disk); -int virDomainDiskDefAssignAddress(virDomainDiskDefPtr def); +int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def); int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller); diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 1968537..f50e090 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -228,6 +228,8 @@ esxCapsInit(esxPrivate *priv) virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x0c, 0x29 }); virCapabilitiesAddHostMigrateTransport(caps, "esx"); + caps->hasWideScsiBus = true; + if (esxLookupHostSystemBiosUuid(priv, caps->host.host_uuid) < 0) { goto failure; } diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index f096876..1b18a5f 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -5019,7 +5019,8 @@ error: * Will fail if not using the 'index' keyword */ static virDomainDiskDefPtr -qemuParseCommandLineDisk(const char *val, +qemuParseCommandLineDisk(virCapsPtr caps, + const char *val, int nvirtiodisk) { virDomainDiskDefPtr def = NULL; @@ -5192,7 +5193,7 @@ qemuParseCommandLineDisk(const char *val, else def->dst[2] = 'a' + idx; - if (virDomainDiskDefAssignAddress(def) < 0) { + if (virDomainDiskDefAssignAddress(caps, def) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("invalid device name '%s'"), def->dst); virDomainDiskDefFree(def); @@ -6004,7 +6005,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, goto no_memory; } - if (virDomainDiskDefAssignAddress(disk) < 0) + if (virDomainDiskDefAssignAddress(caps, disk) < 0) goto error; if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) { @@ -6152,7 +6153,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, } else if (STREQ(arg, "-drive")) { virDomainDiskDefPtr disk; WANT_VALUE(); - if (!(disk = qemuParseCommandLineDisk(val, nvirtiodisk))) + if (!(disk = qemuParseCommandLineDisk(caps, val, nvirtiodisk))) goto error; if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) { virDomainDiskDefFree(disk); -- 1.7.0.4

On Thu, Jun 17, 2010 at 11:15:42PM +0200, Matthias Bolte wrote:
The domain XML parsing code autogenerates disk address and controller elements when they are not explicitly specified. The code assumes a narrow SCSI bus (7 units per bus). ESX uses a wide SCSI bus (16 units per bus).
This is a step towards controller support for the ESX driver. --- src/conf/capabilities.h | 3 +++ src/conf/domain_conf.c | 39 +++++++++++++++++++++++++++++---------- src/conf/domain_conf.h | 2 +- src/esx/esx_driver.c | 2 ++ src/qemu/qemu_conf.c | 9 +++++---- 5 files changed, 40 insertions(+), 15 deletions(-)
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index bdf44fa..9290c82 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -24,6 +24,8 @@ #ifndef __VIR_CAPABILITIES_H # define __VIR_CAPABILITIES_H
+# include <stdbool.h> + # include "internal.h" # include "util.h" # include "buf.h" @@ -125,6 +127,7 @@ struct _virCaps { void (*privateDataFreeFunc)(void *); int (*privateDataXMLFormat)(virBufferPtr, void *); int (*privateDataXMLParse)(xmlXPathContextPtr, void *); + bool hasWideScsiBus; };
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 64b5cf3..cac4042 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1339,7 +1339,7 @@ virDomainParseLegacyDeviceAddress(char *devaddr, }
int -virDomainDiskDefAssignAddress(virDomainDiskDefPtr def) +virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) { int idx = virDiskNameToIndex(def->dst); if (idx < 0) @@ -1347,12 +1347,30 @@ virDomainDiskDefAssignAddress(virDomainDiskDefPtr def)
switch (def->bus) { case VIR_DOMAIN_DISK_BUS_SCSI: - /* For SCSI we define the default mapping to be 7 units - * per bus, 1 bus per controller, many controllers */ def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE; - def->info.addr.drive.controller = idx / 7; - def->info.addr.drive.bus = 0; - def->info.addr.drive.unit = idx % 7; + + if (caps->hasWideScsiBus) { + /* For a wide SCSI bus we define the default mapping to be + * 16 units per bus, 1 bus per controller, many controllers. + * Unit 7 is the SCSI controller itself. Therefore unit 7 + * cannot be assigned to disks and is skipped. + */ + def->info.addr.drive.controller = idx / 15; + def->info.addr.drive.bus = 0; + def->info.addr.drive.unit = idx % 15; + + /* Skip the SCSI controller at unit 7 */ + if (def->info.addr.drive.unit >= 7) { + ++def->info.addr.drive.unit; + } + } else { + /* For a narrow SCSI bus we define the default mapping to be + * 7 units per bus, 1 bus per controller, many controllers */ + def->info.addr.drive.controller = idx / 7; + def->info.addr.drive.bus = 0; + def->info.addr.drive.unit = idx % 7; + } + break;
case VIR_DOMAIN_DISK_BUS_IDE: @@ -1385,7 +1403,8 @@ virDomainDiskDefAssignAddress(virDomainDiskDefPtr def) * @param node XML nodeset to parse for disk definition */ static virDomainDiskDefPtr -virDomainDiskDefParseXML(xmlNodePtr node, +virDomainDiskDefParseXML(virCapsPtr caps, + xmlNodePtr node, int flags) { virDomainDiskDefPtr def; xmlNodePtr cur; @@ -1615,7 +1634,7 @@ virDomainDiskDefParseXML(xmlNodePtr node, serial = NULL;
if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE - && virDomainDiskDefAssignAddress(def) < 0) + && virDomainDiskDefAssignAddress(caps, def) < 0) goto error;
cleanup: @@ -3687,7 +3706,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps,
if (xmlStrEqual(node->name, BAD_CAST "disk")) { dev->type = VIR_DOMAIN_DEVICE_DISK; - if (!(dev->data.disk = virDomainDiskDefParseXML(node, flags))) + if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "filesystem")) { dev->type = VIR_DOMAIN_DEVICE_FS; @@ -4237,7 +4256,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, if (n && VIR_ALLOC_N(def->disks, n) < 0) goto no_memory; for (i = 0 ; i < n ; i++) { - virDomainDiskDefPtr disk = virDomainDiskDefParseXML(nodes[i], + virDomainDiskDefPtr disk = virDomainDiskDefParseXML(caps, nodes[i], flags); if (!disk) goto error; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f83de83..701849f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -999,7 +999,7 @@ int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk); void virDomainDiskInsertPreAlloced(virDomainDefPtr def, virDomainDiskDefPtr disk); -int virDomainDiskDefAssignAddress(virDomainDiskDefPtr def); +int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def);
int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller); diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 1968537..f50e090 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -228,6 +228,8 @@ esxCapsInit(esxPrivate *priv) virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x0c, 0x29 }); virCapabilitiesAddHostMigrateTransport(caps, "esx");
+ caps->hasWideScsiBus = true; + if (esxLookupHostSystemBiosUuid(priv, caps->host.host_uuid) < 0) { goto failure; } diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index f096876..1b18a5f 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -5019,7 +5019,8 @@ error: * Will fail if not using the 'index' keyword */ static virDomainDiskDefPtr -qemuParseCommandLineDisk(const char *val, +qemuParseCommandLineDisk(virCapsPtr caps, + const char *val, int nvirtiodisk) { virDomainDiskDefPtr def = NULL; @@ -5192,7 +5193,7 @@ qemuParseCommandLineDisk(const char *val, else def->dst[2] = 'a' + idx;
- if (virDomainDiskDefAssignAddress(def) < 0) { + if (virDomainDiskDefAssignAddress(caps, def) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("invalid device name '%s'"), def->dst); virDomainDiskDefFree(def); @@ -6004,7 +6005,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, goto no_memory; }
- if (virDomainDiskDefAssignAddress(disk) < 0) + if (virDomainDiskDefAssignAddress(caps, disk) < 0) goto error;
if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) { @@ -6152,7 +6153,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, } else if (STREQ(arg, "-drive")) { virDomainDiskDefPtr disk; WANT_VALUE(); - if (!(disk = qemuParseCommandLineDisk(val, nvirtiodisk))) + if (!(disk = qemuParseCommandLineDisk(caps, val, nvirtiodisk))) goto error; if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) { virDomainDiskDefFree(disk);
That looks fine to me, ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

2010/6/21 Daniel Veillard <veillard@redhat.com>:
On Thu, Jun 17, 2010 at 11:15:42PM +0200, Matthias Bolte wrote:
The domain XML parsing code autogenerates disk address and controller elements when they are not explicitly specified. The code assumes a narrow SCSI bus (7 units per bus). ESX uses a wide SCSI bus (16 units per bus).
This is a step towards controller support for the ESX driver. ---
That looks fine to me,
ACK,
Daniel
Thanks, pushed. Matthias

This is a step towards controller support for the ESX driver. --- docs/schemas/domain.rng | 9 +++++++++ src/conf/domain_conf.c | 34 ++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 11 +++++++++++ src/qemu/qemu_driver.c | 1 + 4 files changed, 55 insertions(+), 0 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 9121da3..6f3408d 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -669,6 +669,15 @@ <ref name="unsignedInt"/> </attribute> <optional> + <attribute name="model"> + <choice> + <value>buslogic</value> + <value>lsilogic</value> + <value>lsisas1068</value> + </choice> + </attribute> + </optional> + <optional> <ref name="address"/> </optional> </element> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cac4042..182d8ab 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -139,6 +139,11 @@ VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, "sata", "virtio-serial") +VIR_ENUM_IMPL(virDomainControllerModel, VIR_DOMAIN_CONTROLLER_MODEL_LAST, + "buslogic", + "lsilogic", + "lsisas1068") + VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST, "mount", "block", @@ -1670,6 +1675,7 @@ virDomainControllerDefParseXML(xmlNodePtr node, virDomainControllerDefPtr def; char *type = NULL; char *idx = NULL; + char *model = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -1694,6 +1700,17 @@ virDomainControllerDefParseXML(xmlNodePtr node, } } + model = virXMLPropString(node, "model"); + if (model) { + if ((def->model = virDomainControllerModelTypeFromString(model)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown model type '%s'"), model); + goto error; + } + } else { + def->model = -1; + } + if (virDomainDeviceInfoParseXML(node, &def->info, flags) < 0) goto error; @@ -1745,6 +1762,7 @@ virDomainControllerDefParseXML(xmlNodePtr node, cleanup: VIR_FREE(type); VIR_FREE(idx); + VIR_FREE(model); return def; @@ -4819,6 +4837,7 @@ static int virDomainDefMaybeAddController(virDomainDefPtr def, cont->type = type; cont->idx = idx; + cont->model = -1; if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL) { cont->opts.vioserial.ports = -1; @@ -5232,6 +5251,7 @@ virDomainControllerDefFormat(virBufferPtr buf, int flags) { const char *type = virDomainControllerTypeToString(def->type); + const char *model = NULL; if (!type) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -5239,10 +5259,24 @@ virDomainControllerDefFormat(virBufferPtr buf, return -1; } + if (def->model != -1) { + model = virDomainControllerModelTypeToString(def->model); + + if (!model) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected model type %d"), def->model); + return -1; + } + } + virBufferVSprintf(buf, " <controller type='%s' index='%d'", type, def->idx); + if (model) { + virBufferEscapeString(buf, " model='%s'", model); + } + switch (def->type) { case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: if (def->opts.vioserial.ports != -1) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 701849f..55893d6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -194,6 +194,15 @@ enum virDomainControllerType { VIR_DOMAIN_CONTROLLER_TYPE_LAST }; + +enum virDomainControllerModel { + VIR_DOMAIN_CONTROLLER_MODEL_BUSLOGIC, + VIR_DOMAIN_CONTROLLER_MODEL_LSILOGIC, + VIR_DOMAIN_CONTROLLER_MODEL_LSISAS1068, + + VIR_DOMAIN_CONTROLLER_MODEL_LAST +}; + typedef struct _virDomainVirtioSerialOpts virDomainVirtioSerialOpts; typedef virDomainVirtioSerialOpts *virDomainVirtioSerialOptsPtr; struct _virDomainVirtioSerialOpts { @@ -207,6 +216,7 @@ typedef virDomainControllerDef *virDomainControllerDefPtr; struct _virDomainControllerDef { int type; int idx; + int model; /* -1 == undef */ union { virDomainVirtioSerialOpts vioserial; } opts; @@ -1073,6 +1083,7 @@ VIR_ENUM_DECL(virDomainDiskBus) VIR_ENUM_DECL(virDomainDiskCache) VIR_ENUM_DECL(virDomainDiskErrorPolicy) VIR_ENUM_DECL(virDomainController) +VIR_ENUM_DECL(virDomainControllerModel) VIR_ENUM_DECL(virDomainFS) VIR_ENUM_DECL(virDomainNet) VIR_ENUM_DECL(virDomainChrTarget) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c7923bc..25caa29 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7260,6 +7260,7 @@ qemuDomainFindOrCreateSCSIDiskController(struct qemud_driver *driver, } cont->type = VIR_DOMAIN_CONTROLLER_TYPE_SCSI; cont->idx = 0; + cont->model = -1; VIR_INFO0("No SCSI controller present, hotplugging one"); if (qemudDomainAttachPciControllerDevice(driver, -- 1.7.0.4

On Thu, Jun 17, 2010 at 11:15:43PM +0200, Matthias Bolte wrote:
This is a step towards controller support for the ESX driver. --- docs/schemas/domain.rng | 9 +++++++++ src/conf/domain_conf.c | 34 ++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 11 +++++++++++ src/qemu/qemu_driver.c | 1 + 4 files changed, 55 insertions(+), 0 deletions(-)
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 9121da3..6f3408d 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -669,6 +669,15 @@ <ref name="unsignedInt"/> </attribute> <optional> + <attribute name="model"> + <choice> + <value>buslogic</value> + <value>lsilogic</value> + <value>lsisas1068</value> + </choice> + </attribute> + </optional> + <optional> <ref name="address"/> </optional> </element> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cac4042..182d8ab 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -139,6 +139,11 @@ VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, "sata", "virtio-serial")
+VIR_ENUM_IMPL(virDomainControllerModel, VIR_DOMAIN_CONTROLLER_MODEL_LAST, + "buslogic", + "lsilogic", + "lsisas1068") + VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST, "mount", "block", @@ -1670,6 +1675,7 @@ virDomainControllerDefParseXML(xmlNodePtr node, virDomainControllerDefPtr def; char *type = NULL; char *idx = NULL; + char *model = NULL;
if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -1694,6 +1700,17 @@ virDomainControllerDefParseXML(xmlNodePtr node, } }
+ model = virXMLPropString(node, "model"); + if (model) { + if ((def->model = virDomainControllerModelTypeFromString(model)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown model type '%s'"), model); + goto error; + } + } else { + def->model = -1; + } + if (virDomainDeviceInfoParseXML(node, &def->info, flags) < 0) goto error;
@@ -1745,6 +1762,7 @@ virDomainControllerDefParseXML(xmlNodePtr node, cleanup: VIR_FREE(type); VIR_FREE(idx); + VIR_FREE(model);
return def;
@@ -4819,6 +4837,7 @@ static int virDomainDefMaybeAddController(virDomainDefPtr def,
cont->type = type; cont->idx = idx; + cont->model = -1;
if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL) { cont->opts.vioserial.ports = -1; @@ -5232,6 +5251,7 @@ virDomainControllerDefFormat(virBufferPtr buf, int flags) { const char *type = virDomainControllerTypeToString(def->type); + const char *model = NULL;
if (!type) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -5239,10 +5259,24 @@ virDomainControllerDefFormat(virBufferPtr buf, return -1; }
+ if (def->model != -1) { + model = virDomainControllerModelTypeToString(def->model); + + if (!model) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected model type %d"), def->model); + return -1; + } + } + virBufferVSprintf(buf, " <controller type='%s' index='%d'", type, def->idx);
+ if (model) { + virBufferEscapeString(buf, " model='%s'", model); + } + switch (def->type) { case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: if (def->opts.vioserial.ports != -1) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 701849f..55893d6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -194,6 +194,15 @@ enum virDomainControllerType { VIR_DOMAIN_CONTROLLER_TYPE_LAST };
+ +enum virDomainControllerModel { + VIR_DOMAIN_CONTROLLER_MODEL_BUSLOGIC, + VIR_DOMAIN_CONTROLLER_MODEL_LSILOGIC, + VIR_DOMAIN_CONTROLLER_MODEL_LSISAS1068, + + VIR_DOMAIN_CONTROLLER_MODEL_LAST +}; + typedef struct _virDomainVirtioSerialOpts virDomainVirtioSerialOpts; typedef virDomainVirtioSerialOpts *virDomainVirtioSerialOptsPtr; struct _virDomainVirtioSerialOpts { @@ -207,6 +216,7 @@ typedef virDomainControllerDef *virDomainControllerDefPtr; struct _virDomainControllerDef { int type; int idx; + int model; /* -1 == undef */ union { virDomainVirtioSerialOpts vioserial; } opts; @@ -1073,6 +1083,7 @@ VIR_ENUM_DECL(virDomainDiskBus) VIR_ENUM_DECL(virDomainDiskCache) VIR_ENUM_DECL(virDomainDiskErrorPolicy) VIR_ENUM_DECL(virDomainController) +VIR_ENUM_DECL(virDomainControllerModel) VIR_ENUM_DECL(virDomainFS) VIR_ENUM_DECL(virDomainNet) VIR_ENUM_DECL(virDomainChrTarget) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c7923bc..25caa29 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7260,6 +7260,7 @@ qemuDomainFindOrCreateSCSIDiskController(struct qemud_driver *driver, } cont->type = VIR_DOMAIN_CONTROLLER_TYPE_SCSI; cont->idx = 0; + cont->model = -1;
VIR_INFO0("No SCSI controller present, hotplugging one"); if (qemudDomainAttachPciControllerDevice(driver,
Looks fine to me but the fack that the attribute is ignored except on ESX driver should probably be documented in some ways ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

2010/6/21 Daniel Veillard <veillard@redhat.com>:
On Thu, Jun 17, 2010 at 11:15:43PM +0200, Matthias Bolte wrote:
This is a step towards controller support for the ESX driver. --- docs/schemas/domain.rng | 9 +++++++++ src/conf/domain_conf.c | 34 ++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 11 +++++++++++ src/qemu/qemu_driver.c | 1 + 4 files changed, 55 insertions(+), 0 deletions(-)
Looks fine to me but the fack that the attribute is ignored except on ESX driver should probably be documented in some ways
Yes, but we'll need to document the controller element first. For now I extended the ESX section to cover it.
ACK,
Daniel
Thanks, pushed. Matthias

Also don't abuse the disk driver name to specify the SCSI controller model anymore: <driver name='buslogic'/> Use the newly added model attribute of the controller element for this: <controller type='scsi' index='0' model='buslogic'/> The disk driver name approach is deprecated now, but still works for backward compatibility reasons. Update the documentation and tests accordingly. Fix usage of the words controller and id in the VMX handling code. Use controller, bus and unit properly. --- docs/drvesx.html.in | 25 +- src/esx/esx_driver.c | 12 +- src/esx/esx_vmx.c | 579 +++++++++++++++------- src/esx/esx_vmx.h | 28 +- tests/vmx2xmldata/vmx2xml-case-insensitive-1.xml | 3 +- tests/vmx2xmldata/vmx2xml-case-insensitive-2.xml | 3 +- tests/vmx2xmldata/vmx2xml-cdrom-ide-device.xml | 2 + tests/vmx2xmldata/vmx2xml-cdrom-ide-file.xml | 2 + tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml | 2 + tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml | 2 + tests/vmx2xmldata/vmx2xml-esx-in-the-wild-1.xml | 3 +- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml | 13 +- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-3.xml | 7 +- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-4.xml | 3 +- tests/vmx2xmldata/vmx2xml-floppy-device.xml | 2 + tests/vmx2xmldata/vmx2xml-floppy-file.xml | 2 + tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-1.xml | 2 + tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-2.xml | 2 + tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-3.xml | 2 + tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-4.xml | 2 + tests/vmx2xmldata/vmx2xml-harddisk-ide-file.xml | 2 + tests/vmx2xmldata/vmx2xml-harddisk-scsi-file.xml | 2 + tests/vmx2xmldata/vmx2xml-scsi-driver.xml | 9 +- tests/vmx2xmldata/vmx2xml-scsi-writethrough.xml | 4 +- tests/vmx2xmltest.c | 60 +++- tests/xml2vmxtest.c | 10 +- 26 files changed, 559 insertions(+), 224 deletions(-) diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in index 3c48c77..bc7e1a0 100644 --- a/docs/drvesx.html.in +++ b/docs/drvesx.html.in @@ -311,6 +311,21 @@ ethernet0.checkMACAddress = "false" <pre> ... <disk type='file' device='disk'> + <source file='[local-storage] Fedora11/Fedora11.vmdk'/> + <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='scsi' index='0' model='<strong>lsilogic</strong>'/> + ... +</pre> + <p> + The controller element is supported <span class="since">since 0.8.2</span>. + Prior to this <code><driver name='lsilogic'/></code> was abused to + specify the SCSI controller model. This attribute usage is deprecated now. + </p> +<pre> + ... + <disk type='file' device='disk'> <driver name='<strong>lsilogic</strong>'/> <source file='[local-storage] Fedora11/Fedora11.vmdk'/> <target dev='sda' bus='scsi'/> @@ -393,7 +408,7 @@ ide0:0.startConnected = "false" ethernet0.present = "true" ethernet0.networkName = "VM Network" ethernet0.addressType = "vpx" -ethernet0.address = "00:50:56:91:48:c7" +ethernet0.generatedAddress = "00:50:56:91:48:c7" chipset.onlineStandby = "false" guestOSAltName = "Red Hat Enterprise Linux 5 (32-Bit)" guestOS = "rhel5" @@ -434,10 +449,11 @@ Enter root password for example.com: <on_crash>destroy</on_crash> <devices> <disk type='file' device='disk'> - <driver name='lsilogic'/> <source file='[local-storage] Fedora11/Fedora11.vmdk'/> <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='scsi' index='0' model='lsilogic'/> <interface type='bridge'> <mac address='00:50:56:91:48:c7'/> <source bridge='VM Network'/> @@ -465,10 +481,11 @@ $ cat > demo.xml << EOF </os> <devices> <disk type='file' device='disk'> - <driver name='lsilogic'/> <source file='[local-storage] Fedora11/Fedora11.vmdk'/> <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='scsi' index='0' model='lsilogic'/> <interface type='bridge'> <mac address='00:50:56:25:48:c7'/> <source bridge='VM Network'/> @@ -517,7 +534,9 @@ ethernet0.address = "00:50:56:25:48:C7" <disk type='file' device='disk'> <source file='[local-storage] Fedora11/Fedora11.vmdk'/> <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='scsi' index='0'/> <interface type='bridge'> <mac address='00:50:56:25:48:c7'/> <source bridge='VM Network'/> diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index f50e090..acf8908 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2153,8 +2153,8 @@ esxDomainDumpXML(virDomainPtr domain, int flags) goto cleanup; } - def = esxVMX_ParseConfig(priv->host, vmx, datastoreName, directoryName, - priv->host->productVersion); + def = esxVMX_ParseConfig(priv->host, priv->caps, vmx, datastoreName, + directoryName, priv->host->productVersion); if (def != NULL) { xml = virDomainDefFormat(def, flags); @@ -2194,7 +2194,7 @@ esxDomainXMLFromNative(virConnectPtr conn, const char *nativeFormat, return NULL; } - def = esxVMX_ParseConfig(priv->host, nativeConfig, "?", "?", + def = esxVMX_ParseConfig(priv->host, priv->caps, nativeConfig, "?", "?", priv->host->productVersion); if (def != NULL) { @@ -2229,7 +2229,8 @@ esxDomainXMLToNative(virConnectPtr conn, const char *nativeFormat, return NULL; } - vmx = esxVMX_FormatConfig(priv->host, def, priv->host->productVersion); + vmx = esxVMX_FormatConfig(priv->host, priv->caps, def, + priv->host->productVersion); virDomainDefFree(def); @@ -2457,7 +2458,8 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml ATTRIBUTE_UNUSED) } /* Build VMX from domain XML */ - vmx = esxVMX_FormatConfig(priv->host, def, priv->host->productVersion); + vmx = esxVMX_FormatConfig(priv->host, priv->caps, def, + priv->host->productVersion); if (vmx == NULL) { goto cleanup; diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c index 675318f..032f5bc 100644 --- a/src/esx/esx_vmx.c +++ b/src/esx/esx_vmx.c @@ -23,6 +23,8 @@ #include <config.h> +#include <c-ctype.h> + #include "internal.h" #include "virterror_internal.h" #include "memory.h" @@ -78,9 +80,9 @@ def->os ################################################################################ ## disks ####################################################################### - scsi[0..3]:[0..6,8..15] -> <controller>:<id> - ide[0..1]:[0..1] -> <controller>:<id> - floppy[0..1] -> <controller> + scsi[0..3]:[0..6,8..15] -> <controller>:<unit> with 1 bus per controller + ide[0..1]:[0..1] -> <bus>:<unit> with 1 controller + floppy[0..1] -> <unit> with 1 controller and 1 bus per controller def->disks[0]... @@ -100,7 +102,7 @@ def->disks[0]... ->device = _DISK_DEVICE_DISK <=> scsi0:0.deviceType = "scsi-hardDisk" # defaults to ? ->bus = _DISK_BUS_SCSI ->src = <value>.vmdk <=> scsi0:0.fileName = "<value>.vmdk" -->dst = sd[<controller> * 15 + <id> mapped to [a-z]+] +->dst = sd[<controller> * 15 + <unit> mapped to [a-z]+] ->driverName = <driver> <=> scsi0.virtualDev = "<driver>" # default depends on guestOS value ->driverType ->cachemode <=> scsi0:0.writeThrough = "<value>" # defaults to false, true -> _DISK_CACHE_WRITETHRU, false _DISK_CACHE_DEFAULT @@ -124,7 +126,7 @@ def->disks[0]... ->device = _DISK_DEVICE_DISK <=> ide0:0.deviceType = "ata-hardDisk" # defaults to ? ->bus = _DISK_BUS_IDE ->src = <value>.vmdk <=> ide0:0.fileName = "<value>.vmdk" -->dst = hd[<controller> * 2 + <id> mapped to [a-z]+] +->dst = hd[<bus> * 2 + <unit> mapped to [a-z]+] ->driverName ->driverType ->cachemode <=> ide0:0.writeThrough = "<value>" # defaults to false, true -> _DISK_CACHE_WRITETHRU, false _DISK_CACHE_DEFAULT @@ -144,7 +146,7 @@ def->disks[0]... ->device = _DISK_DEVICE_CDROM <=> scsi0:0.deviceType = "cdrom-image" # defaults to ? ->bus = _DISK_BUS_SCSI ->src = <value>.iso <=> scsi0:0.fileName = "<value>.iso" -->dst = sd[<controller> * 15 + <id> mapped to [a-z]+] +->dst = sd[<controller> * 15 + <unit> mapped to [a-z]+] ->driverName = <driver> <=> scsi0.virtualDev = "<driver>" # default depends on guestOS value ->driverType ->cachemode @@ -163,7 +165,7 @@ def->disks[0]... ->device = _DISK_DEVICE_CDROM <=> ide0:0.deviceType = "cdrom-image" # defaults to ? ->bus = _DISK_BUS_IDE ->src = <value>.iso <=> ide0:0.fileName = "<value>.iso" -->dst = hd[<controller> * 2 + <id> mapped to [a-z]+] +->dst = hd[<bus> * 2 + <unit> mapped to [a-z]+] ->driverName ->driverType ->cachemode @@ -183,7 +185,7 @@ def->disks[0]... ->device = _DISK_DEVICE_CDROM <=> scsi0:0.deviceType = "atapi-cdrom" # defaults to ? ->bus = _DISK_BUS_SCSI ->src = <value> <=> scsi0:0.fileName = "<value>" # e.g. "/dev/scd0" ? -->dst = sd[<controller> * 16 + <id> mapped to [a-z]+] +->dst = sd[<controller> * 15 + <unit> mapped to [a-z]+] ->driverName = <driver> <=> scsi0.virtualDev = "<driver>" # default depends on guestOS value ->driverType ->cachemode @@ -203,7 +205,7 @@ def->disks[0]... ->device = _DISK_DEVICE_CDROM <=> ide0:0.deviceType = "atapi-cdrom" # defaults to ? ->bus = _DISK_BUS_IDE ->src = <value> <=> ide0:0.fileName = "<value>" # e.g. "/dev/scd0" -->dst = hd[<controller> * 2 + <id> mapped to [a-z]+] +->dst = hd[<bus> * 2 + <unit> mapped to [a-z]+] ->driverName ->driverType ->cachemode @@ -223,7 +225,7 @@ def->disks[0]... ->device = _DISK_DEVICE_FLOPPY ->bus = _DISK_BUS_FDC ->src = <value>.flp <=> floppy0.fileName = "<value>.flp" -->dst = fd[<controller> mapped to [a-z]+] +->dst = fd[<unit> mapped to [a-z]+] ->driverName ->driverType ->cachemode @@ -243,7 +245,7 @@ def->disks[0]... ->device = _DISK_DEVICE_FLOPPY ->bus = _DISK_BUS_FDC ->src = <value> <=> floppy0.fileName = "<value>" # e.g. "/dev/fd0" -->dst = fd[<controller> mapped to [a-z]+] +->dst = fd[<unit> mapped to [a-z]+] ->driverName ->driverType ->cachemode @@ -429,7 +431,7 @@ def->parallels[0]... int -esxVMX_SCSIDiskNameToControllerAndID(const char *name, int *controller, int *id) +esxVMX_SCSIDiskNameToControllerAndUnit(const char *name, int *controller, int *unit) { int idx; @@ -448,7 +450,7 @@ esxVMX_SCSIDiskNameToControllerAndID(const char *name, int *controller, int *id) return -1; } - /* Each of the 4 SCSI controllers offers 15 IDs for devices */ + /* Each of the 4 SCSI controllers has 1 bus with 15 units each for devices */ if (idx >= (4 * 15)) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("SCSI disk index (parsed from '%s') is too large"), name); @@ -456,11 +458,11 @@ esxVMX_SCSIDiskNameToControllerAndID(const char *name, int *controller, int *id) } *controller = idx / 15; - *id = idx % 15; + *unit = idx % 15; - /* Skip the controller ifself with ID 7 */ - if (*id >= 7) { - ++(*id); + /* Skip the controller ifself at unit 7 */ + if (*unit >= 7) { + ++(*unit); } return 0; @@ -469,7 +471,7 @@ esxVMX_SCSIDiskNameToControllerAndID(const char *name, int *controller, int *id) int -esxVMX_IDEDiskNameToControllerAndID(const char *name, int *controller, int *id) +esxVMX_IDEDiskNameToBusAndUnit(const char *name, int *bus, int *unit) { int idx; @@ -488,15 +490,15 @@ esxVMX_IDEDiskNameToControllerAndID(const char *name, int *controller, int *id) return -1; } - /* Each of the 2 IDE controllers offers 2 IDs for devices */ + /* The IDE controller has 2 buses with 2 units each for devices */ if (idx >= (2 * 2)) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("IDE disk index (parsed from '%s') is too large"), name); return -1; } - *controller = idx / 2; - *id = idx % 2; + *bus = idx / 2; + *unit = idx % 2; return 0; } @@ -504,7 +506,7 @@ esxVMX_IDEDiskNameToControllerAndID(const char *name, int *controller, int *id) int -esxVMX_FloppyDiskNameToController(const char *name, int *controller) +esxVMX_FloppyDiskNameToUnit(const char *name, int *unit) { int idx; @@ -523,13 +525,125 @@ esxVMX_FloppyDiskNameToController(const char *name, int *controller) return -1; } + /* The FDC controller has 1 bus with 2 units for devices */ if (idx >= 2) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Floppy disk index (parsed from '%s') is too large"), name); return -1; } - *controller = idx; + *unit = idx; + + return 0; +} + + + +int +esxVMX_VerifyDiskAddress(virCapsPtr caps, virDomainDiskDefPtr disk) +{ + virDomainDiskDef def; + virDomainDeviceDriveAddressPtr drive; + + memset(&def, 0, sizeof(def)); + + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Unsupported disk address type '%s'"), + virDomainDeviceAddressTypeToString(disk->info.type)); + return -1; + } + + drive = &disk->info.addr.drive; + + def.dst = disk->dst; + def.bus = disk->bus; + + if (virDomainDiskDefAssignAddress(caps, &def) < 0) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not verify disk address")); + return -1; + } + + if (def.info.addr.drive.controller != drive->controller || + def.info.addr.drive.bus != drive->bus || + def.info.addr.drive.unit != drive->unit) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Disk address %d:%d:%d doesn't match target device '%s'"), + drive->controller, drive->bus, drive->unit, disk->dst); + return -1; + } + + /* drive->{controller|bus|unit} is unsigned, no >= 0 checks are necessary */ + if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) { + if (drive->controller > 3) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("SCSI controller index %d out of [0..3] range"), + drive->controller); + return -1; + } + + if (drive->bus != 0) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("SCSI bus index %d out of [0] range"), + drive->bus); + return -1; + } + + if (drive->unit > 15 || drive->unit == 7) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("SCSI unit index %d out of [0..6,8..15] range"), + drive->unit); + return -1; + } + } else if (disk->bus == VIR_DOMAIN_DISK_BUS_IDE) { + if (drive->controller != 0) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("IDE controller index %d out of [0] range"), + drive->controller); + return -1; + } + + if (drive->bus > 1) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("IDE bus index %d out of [0..1] range"), + drive->bus); + return -1; + } + + if (drive->unit > 1) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("IDE unit index %d out of [0..1] range"), + drive->unit); + return -1; + } + } else if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) { + if (drive->controller != 0) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("FDC controller index %d out of [0] range"), + drive->controller); + return -1; + } + + if (drive->bus != 0) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("FDC bus index %d out of [0] range"), + drive->bus); + return -1; + } + + if (drive->unit > 1) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("FDC unit index %d out of [0..1] range"), + drive->unit); + return -1; + } + } else { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Unsupported bus type '%s'"), + virDomainDiskBusTypeToString(disk->bus)); + return -1; + } return 0; } @@ -537,13 +651,69 @@ esxVMX_FloppyDiskNameToController(const char *name, int *controller) int -esxVMX_GatherSCSIControllers(virDomainDefPtr def, char *virtualDev[4], - int present[4]) +esxVMX_HandleLegacySCSIDiskDriverName(virDomainDefPtr def, + virDomainDiskDefPtr disk) { + char *tmp; + int model, i; + virDomainControllerDefPtr controller = NULL; + + if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI || disk->driverName == NULL) { + return 0; + } + + tmp = disk->driverName; + + for (; *tmp != '\0'; ++tmp) { + *tmp = c_tolower(*tmp); + } + + model = virDomainControllerModelTypeFromString(disk->driverName); + + if (model < 0) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Unknown driver name '%s'"), disk->driverName); + return -1; + } + + for (i = 0; i < def->ncontrollers; ++i) { + if (def->controllers[i]->idx == disk->info.addr.drive.controller) { + controller = def->controllers[i]; + break; + } + } + + if (controller == NULL) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Missing SCSI controller for index %d"), + disk->info.addr.drive.controller); + return -1; + } + + if (controller->model == -1) { + controller->model = model; + } else if (controller->model != model) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Inconsistent SCSI controller model ('%s' is not '%s') " + "for SCSI controller index %d"), disk->driverName, + virDomainControllerModelTypeToString(controller->model), + controller->idx); + return -1; + } + + return 0; +} + + + +int +esxVMX_GatherSCSIControllers(virDomainDefPtr def, int virtualDev[4], + bool present[4]) +{ + int i; virDomainDiskDefPtr disk; - int i, controller, id; + virDomainControllerDefPtr controller = NULL; - /* Check for continuous use of the same virtualDev per SCSI controller */ for (i = 0; i < def->ndisks; ++i) { disk = def->disks[i]; @@ -551,34 +721,36 @@ esxVMX_GatherSCSIControllers(virDomainDefPtr def, char *virtualDev[4], continue; } - if (disk->driverName != NULL && - STRCASENEQ(disk->driverName, "buslogic") && - STRCASENEQ(disk->driverName, "lsilogic") && - STRCASENEQ(disk->driverName, "lsisas1068")) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Expecting domain XML entry 'devices/disk/target' to " - "be 'buslogic' or 'lsilogic' or 'lsisas1068' but found '%s'"), - disk->driverName); - return -1; + controller = NULL; + + for (i = 0; i < def->ncontrollers; ++i) { + if (def->controllers[i]->idx == disk->info.addr.drive.controller) { + controller = def->controllers[i]; + break; + } } - if (esxVMX_SCSIDiskNameToControllerAndID(disk->dst, &controller, - &id) < 0) { + if (controller == NULL) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Missing SCSI controller for index %d"), + disk->info.addr.drive.controller); return -1; } - present[controller] = 1; - - if (virtualDev[controller] == NULL) { - virtualDev[controller] = disk->driverName; - } else if (disk->driverName == NULL || - STRCASENEQ(virtualDev[controller], disk->driverName)) { + if (controller->model != -1 && + controller->model != VIR_DOMAIN_CONTROLLER_MODEL_BUSLOGIC && + controller->model != VIR_DOMAIN_CONTROLLER_MODEL_LSILOGIC && + controller->model != VIR_DOMAIN_CONTROLLER_MODEL_LSISAS1068) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Inconsistent driver usage ('%s' is not '%s') on SCSI " - "controller index %d"), virtualDev[controller], - disk->driverName ? disk->driverName : "?", controller); + _("Expecting domain XML attribute 'model' of entry " + "'controller' to be 'buslogic' or 'lsilogic' or " + "'lsisas1068' but found '%s'"), + virDomainControllerModelTypeToString(controller->model)); return -1; } + + present[controller->idx] = true; + virtualDev[controller->idx] = controller->model; } return 0; @@ -725,7 +897,7 @@ esxVMX_ParseFileName(esxVI_Context *ctx, const char *fileName, virDomainDefPtr -esxVMX_ParseConfig(esxVI_Context *ctx, const char *vmx, +esxVMX_ParseConfig(esxVI_Context *ctx, virCapsPtr caps, const char *vmx, const char *datastoreName, const char *directoryName, esxVI_ProductVersion productVersion) { @@ -740,10 +912,11 @@ esxVMX_ParseConfig(esxVI_Context *ctx, const char *vmx, char *sched_cpu_affinity = NULL; char *guestOS = NULL; int controller; + int bus; int port; int present; // boolean - char *scsi_virtualDev = NULL; - int id; + int scsi_virtualDev[4] = { -1, -1, -1, -1 }; + int unit; conf = virConfReadMem(vmx, strlen(vmx), VIR_CONF_FLAG_VMX_FORMAT); @@ -970,12 +1143,11 @@ esxVMX_ParseConfig(esxVI_Context *ctx, const char *vmx, goto cleanup; } -/* - def->emulator - def->features*/ + /* def:features */ + /* FIXME */ -/* - def->localtime*/ + /* def:clock */ + /* FIXME */ /* def:graphics */ if (VIR_ALLOC_N(def->graphics, 1) < 0) { @@ -1003,10 +1175,8 @@ esxVMX_ParseConfig(esxVI_Context *ctx, const char *vmx, /* def:disks (scsi) */ for (controller = 0; controller < 4; ++controller) { - VIR_FREE(scsi_virtualDev); - if (esxVMX_ParseSCSIController(conf, controller, &present, - &scsi_virtualDev) < 0) { + &scsi_virtualDev[controller]) < 0) { goto cleanup; } @@ -1014,18 +1184,18 @@ esxVMX_ParseConfig(esxVI_Context *ctx, const char *vmx, continue; } - for (id = 0; id < 16; ++id) { - if (id == 7) { + for (unit = 0; unit < 16; ++unit) { + if (unit == 7) { /* - * SCSI ID 7 is assigned to the SCSI controller and cannot be + * SCSI unit 7 is assigned to the SCSI controller and cannot be * used for disk devices. */ continue; } - if (esxVMX_ParseDisk(ctx, conf, VIR_DOMAIN_DISK_DEVICE_DISK, - VIR_DOMAIN_DISK_BUS_SCSI, controller, id, - scsi_virtualDev, datastoreName, directoryName, + if (esxVMX_ParseDisk(ctx, caps, conf, VIR_DOMAIN_DISK_DEVICE_DISK, + VIR_DOMAIN_DISK_BUS_SCSI, controller, unit, + datastoreName, directoryName, &def->disks[def->ndisks]) < 0) { goto cleanup; } @@ -1035,9 +1205,9 @@ esxVMX_ParseConfig(esxVI_Context *ctx, const char *vmx, continue; } - if (esxVMX_ParseDisk(ctx, conf, VIR_DOMAIN_DISK_DEVICE_CDROM, - VIR_DOMAIN_DISK_BUS_SCSI, controller, id, - scsi_virtualDev, datastoreName, directoryName, + if (esxVMX_ParseDisk(ctx, caps, conf, VIR_DOMAIN_DISK_DEVICE_CDROM, + VIR_DOMAIN_DISK_BUS_SCSI, controller, unit, + datastoreName, directoryName, &def->disks[def->ndisks]) < 0) { goto cleanup; } @@ -1049,11 +1219,11 @@ esxVMX_ParseConfig(esxVI_Context *ctx, const char *vmx, } /* def:disks (ide) */ - for (controller = 0; controller < 2; ++controller) { - for (id = 0; id < 2; ++id) { - if (esxVMX_ParseDisk(ctx, conf, VIR_DOMAIN_DISK_DEVICE_DISK, - VIR_DOMAIN_DISK_BUS_IDE, controller, id, - NULL, datastoreName, directoryName, + for (bus = 0; bus < 2; ++bus) { + for (unit = 0; unit < 2; ++unit) { + if (esxVMX_ParseDisk(ctx, caps, conf, VIR_DOMAIN_DISK_DEVICE_DISK, + VIR_DOMAIN_DISK_BUS_IDE, bus, unit, + datastoreName, directoryName, &def->disks[def->ndisks]) < 0) { goto cleanup; } @@ -1063,9 +1233,9 @@ esxVMX_ParseConfig(esxVI_Context *ctx, const char *vmx, continue; } - if (esxVMX_ParseDisk(ctx, conf, VIR_DOMAIN_DISK_DEVICE_CDROM, - VIR_DOMAIN_DISK_BUS_IDE, controller, id, - NULL, datastoreName, directoryName, + if (esxVMX_ParseDisk(ctx, caps, conf, VIR_DOMAIN_DISK_DEVICE_CDROM, + VIR_DOMAIN_DISK_BUS_IDE, bus, unit, + datastoreName, directoryName, &def->disks[def->ndisks]) < 0) { goto cleanup; } @@ -1077,9 +1247,9 @@ esxVMX_ParseConfig(esxVI_Context *ctx, const char *vmx, } /* def:disks (floppy) */ - for (controller = 0; controller < 2; ++controller) { - if (esxVMX_ParseDisk(ctx, conf, VIR_DOMAIN_DISK_DEVICE_FLOPPY, - VIR_DOMAIN_DISK_BUS_FDC, controller, -1, NULL, + for (unit = 0; unit < 2; ++unit) { + if (esxVMX_ParseDisk(ctx, caps, conf, VIR_DOMAIN_DISK_DEVICE_FLOPPY, + VIR_DOMAIN_DISK_BUS_FDC, 0, unit, datastoreName, directoryName, &def->disks[def->ndisks]) < 0) { goto cleanup; @@ -1090,6 +1260,27 @@ esxVMX_ParseConfig(esxVI_Context *ctx, const char *vmx, } } + /* def:controllers */ + if (virDomainDefAddImplicitControllers(def) < 0) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add controllers")); + goto cleanup; + } + + for (controller = 0; controller < def->ncontrollers; ++controller) { + if (def->controllers[controller]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { + if (def->controllers[controller]->idx < 0 || + def->controllers[controller]->idx > 3) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("SCSI controller index %d out of [0..3] range"), + def->controllers[controller]->idx); + goto cleanup; + } + + def->controllers[controller]->model = + scsi_virtualDev[def->controllers[controller]->idx]; + } + } + /* def:fss */ /* FIXME */ @@ -1130,8 +1321,7 @@ esxVMX_ParseConfig(esxVI_Context *ctx, const char *vmx, def->nserials = 0; for (port = 0; port < 4; ++port) { - if (esxVMX_ParseSerial(ctx, conf, port, datastoreName, - directoryName, + if (esxVMX_ParseSerial(ctx, conf, port, datastoreName, directoryName, &def->serials[def->nserials]) < 0) { goto cleanup; } @@ -1150,8 +1340,7 @@ esxVMX_ParseConfig(esxVI_Context *ctx, const char *vmx, def->nparallels = 0; for (port = 0; port < 3; ++port) { - if (esxVMX_ParseParallel(ctx, conf, port, datastoreName, - directoryName, + if (esxVMX_ParseParallel(ctx, conf, port, datastoreName, directoryName, &def->parallels[def->nparallels]) < 0) { goto cleanup; } @@ -1172,7 +1361,6 @@ esxVMX_ParseConfig(esxVI_Context *ctx, const char *vmx, virConfFree(conf); VIR_FREE(sched_cpu_affinity); VIR_FREE(guestOS); - VIR_FREE(scsi_virtualDev); return def; } @@ -1245,12 +1433,14 @@ esxVMX_ParseVNC(virConfPtr conf, virDomainGraphicsDefPtr *def) int esxVMX_ParseSCSIController(virConfPtr conf, int controller, int *present, - char **virtualDev) + int *virtualDev) { char present_name[32]; char virtualDev_name[32]; + char *virtualDev_string = NULL; + char *tmp; - if (virtualDev == NULL || *virtualDev != NULL) { + if (virtualDev == NULL || *virtualDev != -1) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); return -1; } @@ -1274,24 +1464,36 @@ esxVMX_ParseSCSIController(virConfPtr conf, int controller, int *present, return 0; } - if (esxUtil_GetConfigString(conf, virtualDev_name, virtualDev, 1) < 0) { + if (esxUtil_GetConfigString(conf, virtualDev_name, &virtualDev_string, + 1) < 0) { goto failure; } - if (*virtualDev != NULL && - STRCASENEQ(*virtualDev, "buslogic") && - STRCASENEQ(*virtualDev, "lsilogic") && - STRCASENEQ(*virtualDev, "lsisas1068")) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Expecting VMX entry '%s' to be 'buslogic' or 'lsilogic' or " - "'lsisas1068' but found '%s'"), virtualDev_name, *virtualDev); - goto failure; + if (virtualDev_string != NULL) { + tmp = virtualDev_string; + + for (; *tmp != '\0'; ++tmp) { + *tmp = c_tolower(*tmp); + } + + *virtualDev = virDomainControllerModelTypeFromString(virtualDev_string); + + if (*virtualDev == -1 || + (*virtualDev != VIR_DOMAIN_CONTROLLER_MODEL_BUSLOGIC && + *virtualDev != VIR_DOMAIN_CONTROLLER_MODEL_LSILOGIC && + *virtualDev != VIR_DOMAIN_CONTROLLER_MODEL_LSISAS1068)) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Expecting VMX entry '%s' to be 'buslogic' or 'lsilogic' " + "or 'lsisas1068' but found '%s'"), virtualDev_name, + virtualDev_string); + goto failure; + } } return 0; failure: - VIR_FREE(*virtualDev); + VIR_FREE(virtualDev_string); return -1; } @@ -1314,29 +1516,26 @@ struct _virDomainDiskDef { };*/ int -esxVMX_ParseDisk(esxVI_Context *ctx, virConfPtr conf, int device, int bus, - int controller, int id, const char *virtualDev, +esxVMX_ParseDisk(esxVI_Context *ctx, virCapsPtr caps, virConfPtr conf, + int device, int busType, int controllerOrBus, int unit, const char *datastoreName, const char *directoryName, virDomainDiskDefPtr *def) { /* - * device = {VIR_DOMAIN_DISK_DEVICE_DISK, VIR_DOMAIN_DISK_DEVICE_CDROM} - * bus = VIR_DOMAIN_DISK_BUS_SCSI - * controller = [0..3] - * id = [0..6,8..15] - * virtualDev = {'buslogic', 'lsilogic', 'lsisas1068'} + * device = {VIR_DOMAIN_DISK_DEVICE_DISK, VIR_DOMAIN_DISK_DEVICE_CDROM} + * busType = VIR_DOMAIN_DISK_BUS_SCSI + * controllerOrBus = [0..3] -> controller + * unit = [0..6,8..15] * - * device = {VIR_DOMAIN_DISK_DEVICE_DISK, VIR_DOMAIN_DISK_DEVICE_CDROM} - * bus = VIR_DOMAIN_DISK_BUS_IDE - * controller = [0..1] - * id = [0..1] - * virtualDev = NULL + * device = {VIR_DOMAIN_DISK_DEVICE_DISK, VIR_DOMAIN_DISK_DEVICE_CDROM} + * busType = VIR_DOMAIN_DISK_BUS_IDE + * controllerOrBus = [0..1] -> bus + * unit = [0..1] * - * device = VIR_DOMAIN_DISK_DEVICE_FLOPPY - * bus = VIR_DOMAIN_DISK_BUS_FDC - * controller = [0..1] - * id = -1 - * virtualDev = NULL + * device = VIR_DOMAIN_DISK_DEVICE_FLOPPY + * busType = VIR_DOMAIN_DISK_BUS_FDC + * controllerOrBus = [0] + * unit = [0..1] */ int result = -1; @@ -1374,66 +1573,58 @@ esxVMX_ParseDisk(esxVI_Context *ctx, virConfPtr conf, int device, int bus, } (*def)->device = device; - (*def)->bus = bus; + (*def)->bus = busType; /* def:dst, def:driverName */ if (device == VIR_DOMAIN_DISK_DEVICE_DISK || device == VIR_DOMAIN_DISK_DEVICE_CDROM) { - if (bus == VIR_DOMAIN_DISK_BUS_SCSI) { - if (controller < 0 || controller > 3) { + if (busType == VIR_DOMAIN_DISK_BUS_SCSI) { + if (controllerOrBus < 0 || controllerOrBus > 3) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("SCSI controller index %d out of [0..3] range"), - controller); + controllerOrBus); goto cleanup; } - if (id < 0 || id > 15 || id == 7) { + if (unit < 0 || unit > 15 || unit == 7) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("SCSI ID %d out of [0..6,8..15] range"), id); + _("SCSI unit index %d out of [0..6,8..15] range"), + unit); goto cleanup; } - if (virAsprintf(&prefix, "scsi%d:%d", controller, id) < 0) { + if (virAsprintf(&prefix, "scsi%d:%d", controllerOrBus, unit) < 0) { virReportOOMError(); goto cleanup; } (*def)->dst = virIndexToDiskName - (controller * 15 + (id < 7 ? id : id - 1), "sd"); + (controllerOrBus * 15 + (unit < 7 ? unit : unit - 1), "sd"); if ((*def)->dst == NULL) { goto cleanup; } - - if (virtualDev != NULL) { - (*def)->driverName = strdup(virtualDev); - - if ((*def)->driverName == NULL) { - virReportOOMError(); - goto cleanup; - } - } - } else if (bus == VIR_DOMAIN_DISK_BUS_IDE) { - if (controller < 0 || controller > 1) { + } else if (busType == VIR_DOMAIN_DISK_BUS_IDE) { + if (controllerOrBus < 0 || controllerOrBus > 1) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("IDE controller index %d out of [0..1] range"), - controller); + _("IDE bus index %d out of [0..1] range"), + controllerOrBus); goto cleanup; } - if (id < 0 || id > 1) { + if (unit < 0 || unit > 1) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("IDE ID %d out of [0..1] range"), id); + _("IDE unit index %d out of [0..1] range"), unit); goto cleanup; } - if (virAsprintf(&prefix, "ide%d:%d", controller, id) < 0) { + if (virAsprintf(&prefix, "ide%d:%d", controllerOrBus, unit) < 0) { virReportOOMError(); goto cleanup; } - (*def)->dst = virIndexToDiskName(controller * 2 + id, "hd"); + (*def)->dst = virIndexToDiskName(controllerOrBus * 2 + unit, "hd"); if ((*def)->dst == NULL) { goto cleanup; @@ -1441,25 +1632,32 @@ esxVMX_ParseDisk(esxVI_Context *ctx, virConfPtr conf, int device, int bus, } else { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Unsupported bus type '%s' for device type '%s'"), - virDomainDiskBusTypeToString(bus), + virDomainDiskBusTypeToString(busType), virDomainDiskDeviceTypeToString(device)); goto cleanup; } } else if (device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { - if (bus == VIR_DOMAIN_DISK_BUS_FDC) { - if (controller < 0 || controller > 1) { + if (busType == VIR_DOMAIN_DISK_BUS_FDC) { + if (controllerOrBus != 0) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Floppy controller index %d out of [0..1] range"), - controller); + _("FDC controller index %d out of [0] range"), + controllerOrBus); goto cleanup; } - if (virAsprintf(&prefix, "floppy%d", controller) < 0) { + if (unit < 0 || unit > 1) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("FDC unit index %d out of [0..1] range"), + unit); + goto cleanup; + } + + if (virAsprintf(&prefix, "floppy%d", unit) < 0) { virReportOOMError(); goto cleanup; } - (*def)->dst = virIndexToDiskName(controller, "fd"); + (*def)->dst = virIndexToDiskName(unit, "fd"); if ((*def)->dst == NULL) { goto cleanup; @@ -1467,7 +1665,7 @@ esxVMX_ParseDisk(esxVI_Context *ctx, virConfPtr conf, int device, int bus, } else { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Unsupported bus type '%s' for device type '%s'"), - virDomainDiskBusTypeToString(bus), + virDomainDiskBusTypeToString(busType), virDomainDiskDeviceTypeToString(device)); goto cleanup; } @@ -1541,7 +1739,7 @@ esxVMX_ParseDisk(esxVI_Context *ctx, virConfPtr conf, int device, int bus, if (device == VIR_DOMAIN_DISK_DEVICE_DISK) { if (virFileHasSuffix(fileName, ".vmdk")) { if (deviceType != NULL) { - if (bus == VIR_DOMAIN_DISK_BUS_SCSI && + if (busType == VIR_DOMAIN_DISK_BUS_SCSI && STRCASENEQ(deviceType, "scsi-hardDisk") && STRCASENEQ(deviceType, "disk")) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, @@ -1549,7 +1747,7 @@ esxVMX_ParseDisk(esxVI_Context *ctx, virConfPtr conf, int device, int bus, "or 'disk' but found '%s'"), deviceType_name, deviceType); goto cleanup; - } else if (bus == VIR_DOMAIN_DISK_BUS_IDE && + } else if (busType == VIR_DOMAIN_DISK_BUS_IDE && STRCASENEQ(deviceType, "ata-hardDisk") && STRCASENEQ(deviceType, "disk")) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, @@ -1560,16 +1758,6 @@ esxVMX_ParseDisk(esxVI_Context *ctx, virConfPtr conf, int device, int bus, } } - if (writeThrough && virtualDev == NULL) { - /* - * FIXME: If no virtualDev is explicit specified need to get - * the default based on the guestOS. The mechanism to - * obtain the default is currently missing - */ - VIR_WARN0("No explicit SCSI driver specified in VMX config, " - "cannot represent explicit specified cachemode"); - } - (*def)->type = VIR_DOMAIN_DISK_TYPE_FILE; (*def)->src = esxVMX_ParseFileName(ctx, fileName, datastoreName, directoryName); @@ -1666,6 +1854,12 @@ esxVMX_ParseDisk(esxVI_Context *ctx, virConfPtr conf, int device, int bus, goto cleanup; } + if (virDomainDiskDefAssignAddress(caps, *def) < 0) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Could not assign address to disk '%s'"), (*def)->src); + goto cleanup; + } + result = 0; cleanup: @@ -2239,13 +2433,15 @@ esxVMX_FormatFileName(esxVI_Context *ctx ATTRIBUTE_UNUSED, const char *src) char * -esxVMX_FormatConfig(esxVI_Context *ctx, virDomainDefPtr def, +esxVMX_FormatConfig(esxVI_Context *ctx, virCapsPtr caps, virDomainDefPtr def, esxVI_ProductVersion productVersion) { int i; int sched_cpu_affinity_length; unsigned char zero[VIR_UUID_BUFLEN]; virBuffer buffer = VIR_BUFFER_INITIALIZER; + bool scsi_present[4] = { false, false, false, false }; + int scsi_virtualDev[4] = { -1, -1, -1, -1 }; memset(zero, 0, VIR_UUID_BUFLEN); @@ -2398,8 +2594,12 @@ esxVMX_FormatConfig(esxVI_Context *ctx, virDomainDefPtr def, } /* def:disks */ - int scsi_present[4] = { 0, 0, 0, 0 }; - char *scsi_virtualDev[4] = { NULL, NULL, NULL, NULL }; + for (i = 0; i < def->ndisks; ++i) { + if (esxVMX_VerifyDiskAddress(caps, def->disks[i]) < 0 || + esxVMX_HandleLegacySCSIDiskDriverName(def, def->disks[i]) < 0) { + goto failure; + } + } if (esxVMX_GatherSCSIControllers(def, scsi_virtualDev, scsi_present) < 0) { goto failure; @@ -2409,9 +2609,10 @@ esxVMX_FormatConfig(esxVI_Context *ctx, virDomainDefPtr def, if (scsi_present[i]) { virBufferVSprintf(&buffer, "scsi%d.present = \"true\"\n", i); - if (scsi_virtualDev[i] != NULL) { + if (scsi_virtualDev[i] != -1) { virBufferVSprintf(&buffer, "scsi%d.virtualDev = \"%s\"\n", i, - scsi_virtualDev[i]); + virDomainControllerModelTypeToString + (scsi_virtualDev[i])); } } } @@ -2543,7 +2744,7 @@ int esxVMX_FormatHardDisk(esxVI_Context *ctx, virDomainDiskDefPtr def, virBufferPtr buffer) { - int controller, id; + int controllerOrBus, unit; const char *busName = NULL; const char *entryPrefix = NULL; const char *deviceTypePrefix = NULL; @@ -2559,8 +2760,8 @@ esxVMX_FormatHardDisk(esxVI_Context *ctx, virDomainDiskDefPtr def, entryPrefix = "scsi"; deviceTypePrefix = "scsi"; - if (esxVMX_SCSIDiskNameToControllerAndID(def->dst, &controller, - &id) < 0) { + if (esxVMX_SCSIDiskNameToControllerAndUnit(def->dst, &controllerOrBus, + &unit) < 0) { return -1; } } else if (def->bus == VIR_DOMAIN_DISK_BUS_IDE) { @@ -2568,8 +2769,8 @@ esxVMX_FormatHardDisk(esxVI_Context *ctx, virDomainDiskDefPtr def, entryPrefix = "ide"; deviceTypePrefix = "ata"; - if (esxVMX_IDEDiskNameToControllerAndID(def->dst, &controller, - &id) < 0) { + if (esxVMX_IDEDiskNameToBusAndUnit(def->dst, &controllerOrBus, + &unit) < 0) { return -1; } } else { @@ -2588,9 +2789,9 @@ esxVMX_FormatHardDisk(esxVI_Context *ctx, virDomainDiskDefPtr def, } virBufferVSprintf(buffer, "%s%d:%d.present = \"true\"\n", - entryPrefix, controller, id); + entryPrefix, controllerOrBus, unit); virBufferVSprintf(buffer, "%s%d:%d.deviceType = \"%s-hardDisk\"\n", - entryPrefix, controller, id, deviceTypePrefix); + entryPrefix, controllerOrBus, unit, deviceTypePrefix); if (def->src != NULL) { if (! virFileHasSuffix(def->src, ".vmdk")) { @@ -2607,7 +2808,7 @@ esxVMX_FormatHardDisk(esxVI_Context *ctx, virDomainDiskDefPtr def, } virBufferVSprintf(buffer, "%s%d:%d.fileName = \"%s\"\n", - entryPrefix, controller, id, fileName); + entryPrefix, controllerOrBus, unit, fileName); VIR_FREE(fileName); } @@ -2615,7 +2816,7 @@ esxVMX_FormatHardDisk(esxVI_Context *ctx, virDomainDiskDefPtr def, if (def->bus == VIR_DOMAIN_DISK_BUS_SCSI) { if (def->cachemode == VIR_DOMAIN_DISK_CACHE_WRITETHRU) { virBufferVSprintf(buffer, "%s%d:%d.writeThrough = \"true\"\n", - entryPrefix, controller, id); + entryPrefix, controllerOrBus, unit); } else if (def->cachemode != VIR_DOMAIN_DISK_CACHE_DEFAULT) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("%s harddisk '%s' has unsupported cache mode '%s'"), @@ -2634,7 +2835,7 @@ int esxVMX_FormatCDROM(esxVI_Context *ctx, virDomainDiskDefPtr def, virBufferPtr buffer) { - int controller, id; + int controllerOrBus, unit; const char *busName = NULL; const char *entryPrefix = NULL; char *fileName = NULL; @@ -2648,16 +2849,16 @@ esxVMX_FormatCDROM(esxVI_Context *ctx, virDomainDiskDefPtr def, busName = "SCSI"; entryPrefix = "scsi"; - if (esxVMX_SCSIDiskNameToControllerAndID(def->dst, &controller, - &id) < 0) { + if (esxVMX_SCSIDiskNameToControllerAndUnit(def->dst, &controllerOrBus, + &unit) < 0) { return -1; } } else if (def->bus == VIR_DOMAIN_DISK_BUS_IDE) { busName = "IDE"; entryPrefix = "ide"; - if (esxVMX_IDEDiskNameToControllerAndID(def->dst, &controller, - &id) < 0) { + if (esxVMX_IDEDiskNameToBusAndUnit(def->dst, &controllerOrBus, + &unit) < 0) { return -1; } } else { @@ -2668,11 +2869,11 @@ esxVMX_FormatCDROM(esxVI_Context *ctx, virDomainDiskDefPtr def, } virBufferVSprintf(buffer, "%s%d:%d.present = \"true\"\n", - entryPrefix, controller, id); + entryPrefix, controllerOrBus, unit); if (def->type == VIR_DOMAIN_DISK_TYPE_FILE) { virBufferVSprintf(buffer, "%s%d:%d.deviceType = \"cdrom-image\"\n", - entryPrefix, controller, id); + entryPrefix, controllerOrBus, unit); if (def->src != NULL) { if (! virFileHasSuffix(def->src, ".iso")) { @@ -2689,17 +2890,17 @@ esxVMX_FormatCDROM(esxVI_Context *ctx, virDomainDiskDefPtr def, } virBufferVSprintf(buffer, "%s%d:%d.fileName = \"%s\"\n", - entryPrefix, controller, id, fileName); + entryPrefix, controllerOrBus, unit, fileName); VIR_FREE(fileName); } } else if (def->type == VIR_DOMAIN_DISK_TYPE_BLOCK) { virBufferVSprintf(buffer, "%s%d:%d.deviceType = \"atapi-cdrom\"\n", - entryPrefix, controller, id); + entryPrefix, controllerOrBus, unit); if (def->src != NULL) { virBufferVSprintf(buffer, "%s%d:%d.fileName = \"%s\"\n", - entryPrefix, controller, id, def->src); + entryPrefix, controllerOrBus, unit, def->src); } } else { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, @@ -2720,7 +2921,7 @@ int esxVMX_FormatFloppy(esxVI_Context *ctx, virDomainDiskDefPtr def, virBufferPtr buffer) { - int controller; + int unit; char *fileName = NULL; if (def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { @@ -2728,15 +2929,14 @@ esxVMX_FormatFloppy(esxVI_Context *ctx, virDomainDiskDefPtr def, return -1; } - if (esxVMX_FloppyDiskNameToController(def->dst, &controller) < 0) { + if (esxVMX_FloppyDiskNameToUnit(def->dst, &unit) < 0) { return -1; } - virBufferVSprintf(buffer, "floppy%d.present = \"true\"\n", controller); + virBufferVSprintf(buffer, "floppy%d.present = \"true\"\n", unit); if (def->type == VIR_DOMAIN_DISK_TYPE_FILE) { - virBufferVSprintf(buffer, "floppy%d.fileType = \"file\"\n", - controller); + virBufferVSprintf(buffer, "floppy%d.fileType = \"file\"\n", unit); if (def->src != NULL) { if (! virFileHasSuffix(def->src, ".flp")) { @@ -2753,17 +2953,16 @@ esxVMX_FormatFloppy(esxVI_Context *ctx, virDomainDiskDefPtr def, } virBufferVSprintf(buffer, "floppy%d.fileName = \"%s\"\n", - controller, fileName); + unit, fileName); VIR_FREE(fileName); } } else if (def->type == VIR_DOMAIN_DISK_TYPE_BLOCK) { - virBufferVSprintf(buffer, "floppy%d.fileType = \"device\"\n", - controller); + virBufferVSprintf(buffer, "floppy%d.fileType = \"device\"\n", unit); if (def->src != NULL) { virBufferVSprintf(buffer, "floppy%d.fileName = \"%s\"\n", - controller, def->src); + unit, def->src); } } else { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, diff --git a/src/esx/esx_vmx.h b/src/esx/esx_vmx.h index 081b67d..9b66ab8 100644 --- a/src/esx/esx_vmx.h +++ b/src/esx/esx_vmx.h @@ -29,17 +29,25 @@ # include "esx_vi.h" int -esxVMX_SCSIDiskNameToControllerAndID(const char *name, int *controller, int *id); +esxVMX_SCSIDiskNameToControllerAndUnit(const char *name, int *controller, + int *unit); int -esxVMX_IDEDiskNameToControllerAndID(const char *name, int *controller, int *id); +esxVMX_IDEDiskNameToBusAndUnit(const char *name, int *bus, int *unit); int -esxVMX_FloppyDiskNameToController(const char *name, int *controller); +esxVMX_FloppyDiskNameToUnit(const char *name, int *unit); int -esxVMX_GatherSCSIControllers(virDomainDefPtr conf, char *virtualDev[4], - int present[4]); +esxVMX_VerifyDiskAddress(virCapsPtr caps, virDomainDiskDefPtr disk); + +int +esxVMX_HandleLegacySCSIDiskDriverName(virDomainDefPtr def, + virDomainDiskDefPtr disk); + +int +esxVMX_GatherSCSIControllers(virDomainDefPtr def, int virtualDev[4], + bool present[4]); char * esxVMX_AbsolutePathToDatastoreRelatedPath(esxVI_Context *ctx, @@ -56,7 +64,7 @@ esxVMX_ParseFileName(esxVI_Context *ctx, const char *fileName, const char *datastoreName, const char *directoryName); virDomainDefPtr -esxVMX_ParseConfig(esxVI_Context *ctx, const char *vmx, +esxVMX_ParseConfig(esxVI_Context *ctx, virCapsPtr caps, const char *vmx, const char *datastoreName, const char *directoryName, esxVI_ProductVersion productVersion); @@ -65,11 +73,11 @@ esxVMX_ParseVNC(virConfPtr conf, virDomainGraphicsDefPtr *def); int esxVMX_ParseSCSIController(virConfPtr conf, int controller, int *present, - char **virtualDev); + int *virtualDev); int -esxVMX_ParseDisk(esxVI_Context *ctx, virConfPtr conf, int device, int bus, - int controller, int id, const char *virtualDev, +esxVMX_ParseDisk(esxVI_Context *ctx, virCapsPtr caps, virConfPtr conf, + int device, int busType, int controllerOrBus, int unit, const char *datastoreName, const char *directoryName, virDomainDiskDefPtr *def); int @@ -95,7 +103,7 @@ char * esxVMX_FormatFileName(esxVI_Context *ctx, const char *src); char * -esxVMX_FormatConfig(esxVI_Context *ctx, virDomainDefPtr def, +esxVMX_FormatConfig(esxVI_Context *ctx, virCapsPtr caps, virDomainDefPtr def, esxVI_ProductVersion productVersion); int diff --git a/tests/vmx2xmldata/vmx2xml-case-insensitive-1.xml b/tests/vmx2xmldata/vmx2xml-case-insensitive-1.xml index 3131bb2..b47e128 100644 --- a/tests/vmx2xmldata/vmx2xml-case-insensitive-1.xml +++ b/tests/vmx2xmldata/vmx2xml-case-insensitive-1.xml @@ -13,10 +13,11 @@ <on_crash>destroy</on_crash> <devices> <disk type='file' device='disk'> - <driver name='LSILOGIC'/> <source file='[datastore] directory/FEDORA11.VMDK'/> <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='scsi' index='0' model='lsilogic'/> <interface type='bridge'> <mac address='00:50:56:91:48:c7'/> <source bridge='VM NETWORK'/> diff --git a/tests/vmx2xmldata/vmx2xml-case-insensitive-2.xml b/tests/vmx2xmldata/vmx2xml-case-insensitive-2.xml index 766172f..4974f4e 100644 --- a/tests/vmx2xmldata/vmx2xml-case-insensitive-2.xml +++ b/tests/vmx2xmldata/vmx2xml-case-insensitive-2.xml @@ -13,10 +13,11 @@ <on_crash>destroy</on_crash> <devices> <disk type='file' device='disk'> - <driver name='lsilogic'/> <source file='[datastore] directory/fedora11.vmdk'/> <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='scsi' index='0' model='lsilogic'/> <interface type='bridge'> <mac address='00:50:56:91:48:c7'/> <source bridge='vm network'/> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-device.xml b/tests/vmx2xmldata/vmx2xml-cdrom-ide-device.xml index 12c431e..1905f9b 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-ide-device.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-device.xml @@ -14,6 +14,8 @@ <disk type='block' device='cdrom'> <source dev='/dev/scd0'/> <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='ide' index='0'/> </devices> </domain> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-file.xml b/tests/vmx2xmldata/vmx2xml-cdrom-ide-file.xml index c88d90e..b9cf1f9 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-ide-file.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-file.xml @@ -14,6 +14,8 @@ <disk type='file' device='cdrom'> <source file='[datastore] directory/cdrom.iso'/> <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='ide' index='0'/> </devices> </domain> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml index 55ea5e2..1bb42be 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml @@ -14,6 +14,8 @@ <disk type='block' device='cdrom'> <source dev='/dev/scd0'/> <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='scsi' index='0'/> </devices> </domain> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml index ebd5718..bdcb0b0 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml @@ -14,6 +14,8 @@ <disk type='file' device='cdrom'> <source file='[datastore] directory/cdrom.iso'/> <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='scsi' index='0'/> </devices> </domain> diff --git a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-1.xml b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-1.xml index 7cda8b5..fd50008 100644 --- a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-1.xml +++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-1.xml @@ -13,10 +13,11 @@ <on_crash>destroy</on_crash> <devices> <disk type='file' device='disk'> - <driver name='lsilogic'/> <source file='[datastore] directory/Fedora11.vmdk'/> <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='scsi' index='0' model='lsilogic'/> <interface type='bridge'> <mac address='00:50:56:91:48:c7'/> <source bridge='VM Network'/> diff --git a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml index 95fb40b..e98b679 100644 --- a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml +++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml @@ -13,31 +13,40 @@ <on_crash>destroy</on_crash> <devices> <disk type='file' device='disk'> - <driver name='lsilogic' cache='writethrough'/> + <driver cache='writethrough'/> <source file='[datastore] directory/Debian1.vmdk'/> <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> <disk type='file' device='cdrom'> - <driver name='buslogic'/> <source file='[datastore] directory/Debian1-cdrom.iso'/> <target dev='sdp' bus='scsi'/> + <address type='drive' controller='1' bus='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <source file='/vmimages/tools-isoimages/linux.iso'/> <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> <disk type='block' device='cdrom'> <source dev='/dev/scd0'/> <target dev='hdb' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='1'/> </disk> <disk type='file' device='disk'> <source file='[datastore] directory/Debian1-IDE.vmdk'/> <target dev='hdd' bus='ide'/> + <address type='drive' controller='0' bus='1' unit='1'/> </disk> <disk type='block' device='floppy'> <source dev='/dev/fd0'/> <target dev='fda' bus='fdc'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='scsi' index='0' model='lsilogic'/> + <controller type='scsi' index='1' model='buslogic'/> + <controller type='fdc' index='0'/> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:0c:29:3c:98:3e'/> <source bridge='VM Network'/> diff --git a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-3.xml b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-3.xml index ea30c43..6d18209 100644 --- a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-3.xml +++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-3.xml @@ -13,18 +13,23 @@ <on_crash>destroy</on_crash> <devices> <disk type='file' device='disk'> - <driver name='lsilogic'/> <source file='[datastore] directory/Debian2.vmdk'/> <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <source file='[498076b2-02796c1a-ef5b-000ae484a6a3] Isos/debian-testing-amd64-netinst.iso'/> <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> <disk type='file' device='floppy'> <source file='[498076b2-02796c1a-ef5b-000ae484a6a3] Debian2/dummy.flp'/> <target dev='fdb' bus='fdc'/> + <address type='drive' controller='0' bus='0' unit='1'/> </disk> + <controller type='scsi' index='0' model='lsilogic'/> + <controller type='fdc' index='0'/> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:0c:29:f5:c3:0c'/> <source bridge='VM Network'/> diff --git a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-4.xml b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-4.xml index 3a4c6ae..5b8fbb9 100644 --- a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-4.xml +++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-4.xml @@ -13,10 +13,11 @@ <on_crash>destroy</on_crash> <devices> <disk type='file' device='disk'> - <driver name='lsilogic'/> <source file='[datastore] directory/virtMonServ1.vmdk'/> <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='scsi' index='0' model='lsilogic'/> <interface type='bridge'> <mac address='00:50:56:91:66:d4'/> <source bridge='VM Network'/> diff --git a/tests/vmx2xmldata/vmx2xml-floppy-device.xml b/tests/vmx2xmldata/vmx2xml-floppy-device.xml index 3991f73..4ae16d5 100644 --- a/tests/vmx2xmldata/vmx2xml-floppy-device.xml +++ b/tests/vmx2xmldata/vmx2xml-floppy-device.xml @@ -14,6 +14,8 @@ <disk type='block' device='floppy'> <source dev='/dev/fd0'/> <target dev='fda' bus='fdc'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='fdc' index='0'/> </devices> </domain> diff --git a/tests/vmx2xmldata/vmx2xml-floppy-file.xml b/tests/vmx2xmldata/vmx2xml-floppy-file.xml index 5c2e7e3..5ab538e 100644 --- a/tests/vmx2xmldata/vmx2xml-floppy-file.xml +++ b/tests/vmx2xmldata/vmx2xml-floppy-file.xml @@ -14,6 +14,8 @@ <disk type='file' device='floppy'> <source file='[datastore] directory/floppy.flp'/> <target dev='fda' bus='fdc'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='fdc' index='0'/> </devices> </domain> diff --git a/tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-1.xml b/tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-1.xml index 89a70a4..0c308bc 100644 --- a/tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-1.xml +++ b/tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-1.xml @@ -15,7 +15,9 @@ <disk type='file' device='disk'> <source file='[datastore] directory/Debian-System1-0-cl2.vmdk'/> <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:0c:29:d6:2b:d3'/> <source bridge='net1'/> diff --git a/tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-2.xml b/tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-2.xml index 5f23d60..7b6158f 100644 --- a/tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-2.xml +++ b/tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-2.xml @@ -15,7 +15,9 @@ <disk type='file' device='disk'> <source file='[datastore] directory/Debian-System1-0-cl3.vmdk'/> <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:0c:29:d6:cb:a4'/> <source bridge='net1'/> diff --git a/tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-3.xml b/tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-3.xml index 6f54b27..b926db5 100644 --- a/tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-3.xml +++ b/tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-3.xml @@ -15,7 +15,9 @@ <disk type='file' device='disk'> <source file='[datastore] directory/Debian-System1-0-cl1.vmdk'/> <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:0c:29:c4:be:5a'/> <source bridge='net1'/> diff --git a/tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-4.xml b/tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-4.xml index 5b8d64e..5803f4b 100644 --- a/tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-4.xml +++ b/tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-4.xml @@ -15,7 +15,9 @@ <disk type='file' device='disk'> <source file='[datastore] directory/Debian-System1-0-cl2.vmdk'/> <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:0c:29:c5:e3:5d'/> <source bridge='net2'/> diff --git a/tests/vmx2xmldata/vmx2xml-harddisk-ide-file.xml b/tests/vmx2xmldata/vmx2xml-harddisk-ide-file.xml index 59041bd..7699fbb 100644 --- a/tests/vmx2xmldata/vmx2xml-harddisk-ide-file.xml +++ b/tests/vmx2xmldata/vmx2xml-harddisk-ide-file.xml @@ -14,6 +14,8 @@ <disk type='file' device='disk'> <source file='[datastore] directory/harddisk.vmdk'/> <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='ide' index='0'/> </devices> </domain> diff --git a/tests/vmx2xmldata/vmx2xml-harddisk-scsi-file.xml b/tests/vmx2xmldata/vmx2xml-harddisk-scsi-file.xml index 2609f8c..b04597b 100644 --- a/tests/vmx2xmldata/vmx2xml-harddisk-scsi-file.xml +++ b/tests/vmx2xmldata/vmx2xml-harddisk-scsi-file.xml @@ -14,6 +14,8 @@ <disk type='file' device='disk'> <source file='[datastore] directory/harddisk.vmdk'/> <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='scsi' index='0'/> </devices> </domain> diff --git a/tests/vmx2xmldata/vmx2xml-scsi-driver.xml b/tests/vmx2xmldata/vmx2xml-scsi-driver.xml index 1fa9ac4..d39415d 100644 --- a/tests/vmx2xmldata/vmx2xml-scsi-driver.xml +++ b/tests/vmx2xmldata/vmx2xml-scsi-driver.xml @@ -12,19 +12,22 @@ <on_crash>destroy</on_crash> <devices> <disk type='file' device='disk'> - <driver name='buslogic'/> <source file='[datastore] directory/harddisk1.vmdk'/> <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> <disk type='file' device='disk'> - <driver name='lsilogic'/> <source file='[datastore] directory/harddisk2.vmdk'/> <target dev='sdp' bus='scsi'/> + <address type='drive' controller='1' bus='0' unit='0'/> </disk> <disk type='file' device='disk'> - <driver name='lsisas1068'/> <source file='[datastore] directory/harddisk3.vmdk'/> <target dev='sdae' bus='scsi'/> + <address type='drive' controller='2' bus='0' unit='0'/> </disk> + <controller type='scsi' index='0' model='buslogic'/> + <controller type='scsi' index='1' model='lsilogic'/> + <controller type='scsi' index='2' model='lsisas1068'/> </devices> </domain> diff --git a/tests/vmx2xmldata/vmx2xml-scsi-writethrough.xml b/tests/vmx2xmldata/vmx2xml-scsi-writethrough.xml index f1c4083..66e22ae 100644 --- a/tests/vmx2xmldata/vmx2xml-scsi-writethrough.xml +++ b/tests/vmx2xmldata/vmx2xml-scsi-writethrough.xml @@ -12,9 +12,11 @@ <on_crash>destroy</on_crash> <devices> <disk type='file' device='disk'> - <driver name='buslogic' cache='writethrough'/> + <driver cache='writethrough'/> <source file='[datastore] directory/harddisk.vmdk'/> <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='scsi' index='0' model='buslogic'/> </devices> </domain> diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c index 0a8b99e..f1d2471 100644 --- a/tests/vmx2xmltest.c +++ b/tests/vmx2xmltest.c @@ -13,9 +13,59 @@ static char *progname = NULL; static char *abs_srcdir = NULL; +static virCapsPtr caps = NULL; # define MAX_FILE 4096 +static void +testCapsInit(void) +{ + virCapsGuestPtr guest = NULL; + + caps = virCapabilitiesNew("i686", 1, 1); + + if (caps == NULL) { + return; + } + + virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x0c, 0x29 }); + virCapabilitiesAddHostMigrateTransport(caps, "esx"); + + caps->hasWideScsiBus = true; + + /* i686 guest */ + guest = + virCapabilitiesAddGuest(caps, "hvm", "i686", 32, NULL, NULL, 0, NULL); + + if (guest == NULL) { + goto failure; + } + + if (virCapabilitiesAddGuestDomain(guest, "vmware", NULL, NULL, 0, + NULL) == NULL) { + goto failure; + } + + /* x86_64 guest */ + guest = + virCapabilitiesAddGuest(caps, "hvm", "x86_64", 64, NULL, NULL, 0, NULL); + + if (guest == NULL) { + goto failure; + } + + if (virCapabilitiesAddGuestDomain(guest, "vmware", NULL, NULL, 0, + NULL) == NULL) { + goto failure; + } + + return; + + failure: + virCapabilitiesFree(caps); + caps = NULL; +} + static int testCompareFiles(const char *vmx, const char *xml, esxVI_ProductVersion productVersion) @@ -37,7 +87,7 @@ testCompareFiles(const char *vmx, const char *xml, goto failure; } - def = esxVMX_ParseConfig(NULL, vmxData, "datastore", "directory", + def = esxVMX_ParseConfig(NULL, caps, vmxData, "datastore", "directory", productVersion); if (def == NULL) { @@ -123,6 +173,12 @@ mymain(int argc, char **argv) } \ } while (0) + testCapsInit(); + + if (caps == NULL) { + return EXIT_FAILURE; + } + DO_TEST("case-insensitive-1", "case-insensitive-1", esxVI_ProductVersion_ESX35); DO_TEST("case-insensitive-2", "case-insensitive-2", esxVI_ProductVersion_ESX35); @@ -176,6 +232,8 @@ mymain(int argc, char **argv) DO_TEST("gsx-in-the-wild-3", "gsx-in-the-wild-3", esxVI_ProductVersion_ESX35); DO_TEST("gsx-in-the-wild-4", "gsx-in-the-wild-4", esxVI_ProductVersion_ESX35); + virCapabilitiesFree(caps); + return result == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/xml2vmxtest.c b/tests/xml2vmxtest.c index 14901fa..0a9bc53 100644 --- a/tests/xml2vmxtest.c +++ b/tests/xml2vmxtest.c @@ -18,7 +18,7 @@ static virCapsPtr caps = NULL; # define MAX_FILE 4096 static void -testESXCapsInit(void) +testCapsInit(void) { virCapsGuestPtr guest = NULL; @@ -28,9 +28,11 @@ testESXCapsInit(void) return; } - virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x50, 0x56 }); + virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x0c, 0x29 }); virCapabilitiesAddHostMigrateTransport(caps, "esx"); + caps->hasWideScsiBus = true; + /* i686 guest */ guest = virCapabilitiesAddGuest(caps, "hvm", "i686", 32, NULL, NULL, 0, NULL); @@ -90,7 +92,7 @@ testCompareFiles(const char *xml, const char *vmx, goto failure; } - formatted = esxVMX_FormatConfig(NULL, def, productVersion); + formatted = esxVMX_FormatConfig(NULL, caps, def, productVersion); if (formatted == NULL) { goto failure; @@ -165,7 +167,7 @@ mymain(int argc, char **argv) } \ } while (0) - testESXCapsInit(); + testCapsInit(); if (caps == NULL) { return EXIT_FAILURE; -- 1.7.0.4

On 06/17/2010 03:15 PM, Matthias Bolte wrote:
Also don't abuse the disk driver name to specify the SCSI controller model anymore:
<driver name='buslogic'/>
Use the newly added model attribute of the controller element for this:
<controller type='scsi' index='0' model='buslogic'/>
I don't see any change to docs/schemas/domain.rng for this new XML attribute. Am I missing something, or how do you write an xml file that uses this attribute and still passes validation?
The disk driver name approach is deprecated now, but still works for backward compatibility reasons.
Update the documentation and tests accordingly.
Fix usage of the words controller and id in the VMX handling code. Use controller, bus and unit properly. --- docs/drvesx.html.in | 25 +-
Is the xml addition ESX-specific, or should the new controller attribute also be documented in docs/formatdomain.html.in? Is anything other than model='lsilogic' supported?
@@ -1014,18 +1184,18 @@ esxVMX_ParseConfig(esxVI_Context *ctx, const char *vmx, continue; }
- for (id = 0; id < 16; ++id) { - if (id == 7) { + for (unit = 0; unit < 16; ++unit) { + if (unit == 7) { /* - * SCSI ID 7 is assigned to the SCSI controller and cannot be + * SCSI unit 7 is assigned to the SCSI controller and cannot be * used for disk devices. */ continue;
I've run out of time for reviewing the rest of the patch today (didn't spot anything else obvious above this point), but you may still want to answer my questions and/or respin accordingly. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2010/6/23 Eric Blake <eblake@redhat.com>:
On 06/17/2010 03:15 PM, Matthias Bolte wrote:
Also don't abuse the disk driver name to specify the SCSI controller model anymore:
<driver name='buslogic'/>
Use the newly added model attribute of the controller element for this:
<controller type='scsi' index='0' model='buslogic'/>
I don't see any change to docs/schemas/domain.rng for this new XML attribute. Am I missing something, or how do you write an xml file that uses this attribute and still passes validation?
The model attribute itself was added in a previous patch of this series, including domain.rng extension. This patch adds usage for the previously added attribute. Validation is another problem... Currently the domain XML dumped by the ESX driver doesn't pass validation anyway, e.g. because the driver doesn't output the boot order (which is required for HVM in the domain.rng), because boot order is complicated with ESX and not easy to determine. But that's a different problem.
The disk driver name approach is deprecated now, but still works for backward compatibility reasons.
Update the documentation and tests accordingly.
Fix usage of the words controller and id in the VMX handling code. Use controller, bus and unit properly. --- docs/drvesx.html.in | 25 +-
Is the xml addition ESX-specific, or should the new controller attribute also be documented in docs/formatdomain.html.in? Is anything other than model='lsilogic' supported?
Currently it's only used by ESX, but it should be documented in docs/formatdomain.html.in. The problem here is that docs/formatdomain.html.in currently completely lacks documentation for the controller element, that was initially added for the QEMU driver a while ago. So, I'll have to document the controller element first in order to document the model attribute in a central place. For now I adapted the existing documentation in the ESX section to document the model attribute. Currently the model attribute accepts the 3 already known values buslogic, lsilogic and lsisas1068. There will be at least 2 additions soon. Matthias

On 06/22/2010 04:52 PM, Matthias Bolte wrote:
2010/6/23 Eric Blake <eblake@redhat.com>:
On 06/17/2010 03:15 PM, Matthias Bolte wrote:
Also don't abuse the disk driver name to specify the SCSI controller model anymore:
<driver name='buslogic'/>
Use the newly added model attribute of the controller element for this:
<controller type='scsi' index='0' model='buslogic'/>
I don't see any change to docs/schemas/domain.rng for this new XML attribute. Am I missing something, or how do you write an xml file that uses this attribute and still passes validation?
The model attribute itself was added in a previous patch of this series, including domain.rng extension. This patch adds usage for the previously added attribute.
Serves me right for only looking at 3/3, since 2/3 already had an ack ;) I see it now, and it looks okay. Thanks for clearing up my question.
docs/drvesx.html.in | 25 +-
Is the xml addition ESX-specific, or should the new controller attribute also be documented in docs/formatdomain.html.in? Is anything other than model='lsilogic' supported?
Currently it's only used by ESX, but it should be documented in docs/formatdomain.html.in. The problem here is that docs/formatdomain.html.in currently completely lacks documentation for the controller element, that was initially added for the QEMU driver a while ago.
formatdomain.html.in lacks a number of things, doesn't it ;)
So, I'll have to document the controller element first in order to document the model attribute in a central place. For now I adapted the existing documentation in the ESX section to document the model attribute.
Fair compromise. I then resumed my patch review. A lot of it looks like mechanical renaming, so it wasn't as big as the number of changed lines led me to believe. For example:
+++ b/src/esx/esx_vmx.h @@ -29,17 +29,25 @@ # include "esx_vi.h"
int -esxVMX_SCSIDiskNameToControllerAndID(const char *name, int *controller, int *id); +esxVMX_SCSIDiskNameToControllerAndUnit(const char *name, int *controller, + int *unit);
It may have been nicer to separate the renaming from the actual functionality additions. Oh well; at this point I'm not going to insist that you split it up, now that I've reviewed the whole thing as-is.
+++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-1.xml @@ -13,10 +13,11 @@ <on_crash>destroy</on_crash> <devices> <disk type='file' device='disk'> - <driver name='lsilogic'/> <source file='[datastore] directory/Fedora11.vmdk'/> <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='scsi' index='0' model='lsilogic'/> <interface type='bridge'> <mac address='00:50:56:91:48:c7'/> <source bridge='VM Network'/>
I noticed you converted most of your tests to the new scheme. But shouldn't you leave at least one test on the old scheme (or better yet, create a new test whose sole purpose is to test the old scheme), to ensure you don't break backwards compatibility? At any rate: ACK to this patch, and the formatdomain.html.in cleanups can come as a separate patch later. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2010/6/23 Eric Blake <eblake@redhat.com>:
On 06/22/2010 04:52 PM, Matthias Bolte wrote:
2010/6/23 Eric Blake <eblake@redhat.com>:
On 06/17/2010 03:15 PM, Matthias Bolte wrote:
Also don't abuse the disk driver name to specify the SCSI controller model anymore:
<driver name='buslogic'/>
Use the newly added model attribute of the controller element for this:
<controller type='scsi' index='0' model='buslogic'/>
I don't see any change to docs/schemas/domain.rng for this new XML attribute. Am I missing something, or how do you write an xml file that uses this attribute and still passes validation?
The model attribute itself was added in a previous patch of this series, including domain.rng extension. This patch adds usage for the previously added attribute.
Serves me right for only looking at 3/3, since 2/3 already had an ack ;)
I see it now, and it looks okay. Thanks for clearing up my question.
docs/drvesx.html.in | 25 +-
Is the xml addition ESX-specific, or should the new controller attribute also be documented in docs/formatdomain.html.in? Is anything other than model='lsilogic' supported?
Currently it's only used by ESX, but it should be documented in docs/formatdomain.html.in. The problem here is that docs/formatdomain.html.in currently completely lacks documentation for the controller element, that was initially added for the QEMU driver a while ago.
formatdomain.html.in lacks a number of things, doesn't it ;)
Yes, we're adding new stuff faster than we're documenting it properly.
+++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-1.xml @@ -13,10 +13,11 @@ <on_crash>destroy</on_crash> <devices> <disk type='file' device='disk'> - <driver name='lsilogic'/> <source file='[datastore] directory/Fedora11.vmdk'/> <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <controller type='scsi' index='0' model='lsilogic'/> <interface type='bridge'> <mac address='00:50:56:91:48:c7'/> <source bridge='VM Network'/>
I noticed you converted most of your tests to the new scheme. But shouldn't you leave at least one test on the old scheme (or better yet, create a new test whose sole purpose is to test the old scheme), to ensure you don't break backwards compatibility?
Well, the XML in the VMX-2-XML tests is the output generated by libvirt, so it's correct that all those files use the new syntax. In case of the XML-2-VMX tests the XML files are the input, and here all tests still use the old way to define the SCSI controller model. Actually, I should add XML-2-VMX tests that use the new syntax and I should add some negative tests to make sure disk address verification works correct. But that's stuff for an additional patch.
At any rate:
ACK to this patch, and the formatdomain.html.in cleanups can come as a separate patch later.
Thanks, pushed. Matthias
participants (3)
-
Daniel Veillard
-
Eric Blake
-
Matthias Bolte