[libvirt] [PATCHv4 0/4] VMX: CD-ROM handling improvements
A user came into #virt the other day and was trying to get libvirtd to work with VMware Fusion 5, which is basically the Mac OS X version of VMware Workstation. In helping him out I noticed a few limitations of our VMX parser so I've added support through this patchset. I don't personally have access or own VMware's product. v4: * Half the patchset has been merged at this point * Add back 'auto detect' support via <source startupPolicy='optional'/> * Made device='lun' match device='cdrom' * Patch 1 & 2 are new to the series v3: * Dropped 'auto detect' support from series as it needs more work based on feedback * Added patch to combine virVMXFormatHardDisk and virVMXFormatCDROM into one function. * Converted to <disk type='block' device='lun'> instead of adding a <driver> element to better match the behavior available via QEMU. v2: * Added additional test cases and fixed issues that arose from those *** BLURB HERE *** Doug Goldstein (4): Allow LUN type disks to have no source Allow <source> for type=block to have no dev VMX: Add support for 'auto detect' fileNames VMX: Add a VMWare Fusion 5 configuration for tests docs/schemas/domaincommon.rng | 8 +- src/conf/domain_conf.c | 9 ++- src/vmx/vmx.c | 28 +++++-- .../vmx2xml-cdrom-ide-raw-auto-detect.vmx | 5 ++ .../vmx2xml-cdrom-ide-raw-auto-detect.xml | 24 ++++++ .../vmx2xml-cdrom-scsi-raw-auto-detect.vmx | 6 ++ .../vmx2xml-cdrom-scsi-raw-auto-detect.xml | 24 ++++++ tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx | 88 ++++++++++++++++++++++ tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml | 39 ++++++++++ tests/vmx2xmltest.c | 4 + .../xml2vmx-cdrom-ide-raw-auto-detect.vmx | 14 ++++ .../xml2vmx-cdrom-ide-raw-auto-detect.xml | 14 ++++ .../xml2vmx-cdrom-scsi-raw-auto-detect.vmx | 15 ++++ .../xml2vmx-cdrom-scsi-raw-auto-detect.xml | 14 ++++ tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.vmx | 30 ++++++++ tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml | 41 ++++++++++ tests/xml2vmxtest.c | 4 + 17 files changed, 356 insertions(+), 11 deletions(-) create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.xml create mode 100644 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-auto-detect.vmx create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-auto-detect.xml create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-auto-detect.vmx create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-auto-detect.xml create mode 100644 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.vmx create mode 100644 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml -- 1.8.1.5
CD-ROMs and Floppies are allowed to have no source to imply they are empty or disconnected. Since the LUN type is used for raw CD-ROM access with QEMU (and VMWare in the future), it also needs to allow an empty source when the raw CD-ROM device is disconnected from the domain. --- src/conf/domain_conf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index aed2a9d..a9ff61c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5218,9 +5218,11 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } /* Only CDROM and Floppy devices are allowed missing source path - * to indicate no media present */ + * to indicate no media present. LUN is for raw access CD-ROMs + * that are not attached to a physical device presently */ if (source == NULL && hosts == NULL && !def->srcpool && def->device != VIR_DOMAIN_DISK_DEVICE_CDROM && + def->device != VIR_DOMAIN_DISK_DEVICE_LUN && def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { virReportError(VIR_ERR_NO_SOURCE, target ? "%s" : NULL, target); -- 1.8.1.5
On 09/09/2013 07:48 PM, Doug Goldstein wrote:
CD-ROMs and Floppies are allowed to have no source to imply they are empty or disconnected. Since the LUN type is used for raw CD-ROM access with QEMU (and VMWare in the future), it also needs to allow an empty source when the raw CD-ROM device is disconnected from the domain. --- src/conf/domain_conf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
ACK.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index aed2a9d..a9ff61c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5218,9 +5218,11 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, }
/* Only CDROM and Floppy devices are allowed missing source path - * to indicate no media present */ + * to indicate no media present. LUN is for raw access CD-ROMs + * that are not attached to a physical device presently */ if (source == NULL && hosts == NULL && !def->srcpool && def->device != VIR_DOMAIN_DISK_DEVICE_CDROM && + def->device != VIR_DOMAIN_DISK_DEVICE_LUN && def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { virReportError(VIR_ERR_NO_SOURCE, target ? "%s" : NULL, target);
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Currently the XML parser already allows the following syntax: <disk type='block' device='cdrom'> <source startupPolicy='optional'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> But it did not support writing out the <source> entry without the actual dev value. It would print dev='(null)'. This change allows it to write out XML to match the parser. --- docs/schemas/domaincommon.rng | 8 +++++--- src/conf/domain_conf.c | 5 +++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index ecd3a42..2f596bf 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1140,9 +1140,11 @@ <interleave> <optional> <element name="source"> - <attribute name="dev"> - <ref name="absFilePath"/> - </attribute> + <optional> + <attribute name="dev"> + <ref name="absFilePath"/> + </attribute> + </optional> <optional> <ref name="startupPolicy"/> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a9ff61c..d4b2dfc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14202,8 +14202,9 @@ virDomainDiskSourceDefFormat(virBufferPtr buf, } break; case VIR_DOMAIN_DISK_TYPE_BLOCK: - virBufferEscapeString(buf, " <source dev='%s'", - def->src); + virBufferAddLit(buf, " <source"); + if (def->src) + virBufferEscapeString(buf, " dev='%s'", def->src); if (def->startupPolicy) virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); -- 1.8.1.5
On 09/09/2013 07:48 PM, Doug Goldstein wrote:
Currently the XML parser already allows the following syntax: <disk type='block' device='cdrom'> <source startupPolicy='optional'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk>
But it did not support writing out the <source> entry without the actual dev value. It would print dev='(null)'. This change allows it to write out XML to match the parser. ---
+++ b/src/conf/domain_conf.c @@ -14202,8 +14202,9 @@ virDomainDiskSourceDefFormat(virBufferPtr buf, } break; case VIR_DOMAIN_DISK_TYPE_BLOCK: - virBufferEscapeString(buf, " <source dev='%s'", - def->src);
If def->src is NULL, this is a no-op, rather than printing def='(null)'. Are you sure your commit message is accurate?
+ virBufferAddLit(buf, " <source"); + if (def->src) + virBufferEscapeString(buf, " dev='%s'", def->src);
The 'if' is unnecessary; virBufferEscapeString is a no-op for a NULL argument. However, you are correct that we MUST output "<source" independently from def->src being NULL, otherwise..
if (def->startupPolicy) virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy);
printing the startupPolicy generates invalid XML. Another (pre-existing) case of an unnecessary 'if'. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Sep 16, 2013 at 12:03 PM, Eric Blake <eblake@redhat.com> wrote:
On 09/09/2013 07:48 PM, Doug Goldstein wrote:
Currently the XML parser already allows the following syntax: <disk type='block' device='cdrom'> <source startupPolicy='optional'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk>
But it did not support writing out the <source> entry without the actual dev value. It would print dev='(null)'. This change allows it to write out XML to match the parser. ---
+++ b/src/conf/domain_conf.c @@ -14202,8 +14202,9 @@ virDomainDiskSourceDefFormat(virBufferPtr buf, } break; case VIR_DOMAIN_DISK_TYPE_BLOCK: - virBufferEscapeString(buf, " <source dev='%s'", - def->src);
If def->src is NULL, this is a no-op, rather than printing def='(null)'. Are you sure your commit message is accurate?
The (null) was assumed based on my knowledge of gnulib's *printf helpers. So I guess the commit message was wrong.
+ virBufferAddLit(buf, " <source"); + if (def->src) + virBufferEscapeString(buf, " dev='%s'", def->src);
The 'if' is unnecessary; virBufferEscapeString is a no-op for a NULL argument. However, you are correct that we MUST output "<source" independently from def->src being NULL, otherwise..
A lot of this file does if checks for NULL of the parameters. It seems like we could clean it up a bit with that condition.
if (def->startupPolicy) virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy);
printing the startupPolicy generates invalid XML. Another (pre-existing) case of an unnecessary 'if'.
So you're saying that <source startupPolicy='optional'/> is invalid XML? Or something else? -- Doug Goldstein
On 09/16/2013 02:20 PM, Doug Goldstein wrote:
case VIR_DOMAIN_DISK_TYPE_BLOCK: - virBufferEscapeString(buf, " <source dev='%s'", - def->src);
If def->src is NULL, this is a no-op, rather than printing def='(null)'. Are you sure your commit message is accurate?
The (null) was assumed based on my knowledge of gnulib's *printf helpers. So I guess the commit message was wrong.
virBufferEscapeString() intentionally avoids calling printf for a NULL string - but had we reached printf, you are right that glibc would print "(null)" (and other libc's might SEGV).
+ virBufferAddLit(buf, " <source"); + if (def->src) + virBufferEscapeString(buf, " dev='%s'", def->src);
The 'if' is unnecessary; virBufferEscapeString is a no-op for a NULL argument. However, you are correct that we MUST output "<source" independently from def->src being NULL, otherwise..
A lot of this file does if checks for NULL of the parameters. It seems like we could clean it up a bit with that condition.
Yeah, probably a lot of those places that we can clean. So far, we've been doing it piece-wise as we come across places that are touched for other reasons.
if (def->startupPolicy) virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy);
printing the startupPolicy generates invalid XML. Another (pre-existing) case of an unnecessary 'if'.
So you're saying that <source startupPolicy='optional'/> is invalid XML? Or something else?
For NULL def->src and non-null startupPolicy, the old code would produce: " startupPolicy='...'" and the new code produces "<source startupPolicy='...'" Basically, your patch separates the data that must always be printed ("<source") from the data that is conditional (" dev='...'"). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
VMWare Fusion 5 can set the CD-ROM's device name to be 'auto detect' when using the physical drive via 'cdrom-raw' device type. VMWare will then connect to first available host CD-ROM to the virtual machine upon start up according to VMWare documentation. If no device is available, it appears that the device will remain disconnected. To better model this a CD-ROM that is marked as "auto detect" when in the off state would be modeled as the following with this patch: <disk type='block' device='lun'> <source startupPolicy='optional'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> Once the domain transitions to the powered on state, libvirt can populate the remaining source data with what is connected, if anything. However future power cycles, the domain may not always start with that device attached. --- src/vmx/vmx.c | 28 ++++++++++++++++++---- .../vmx2xml-cdrom-ide-raw-auto-detect.vmx | 5 ++++ .../vmx2xml-cdrom-ide-raw-auto-detect.xml | 24 +++++++++++++++++++ .../vmx2xml-cdrom-scsi-raw-auto-detect.vmx | 6 +++++ .../vmx2xml-cdrom-scsi-raw-auto-detect.xml | 24 +++++++++++++++++++ tests/vmx2xmltest.c | 2 ++ .../xml2vmx-cdrom-ide-raw-auto-detect.vmx | 14 +++++++++++ .../xml2vmx-cdrom-ide-raw-auto-detect.xml | 14 +++++++++++ .../xml2vmx-cdrom-scsi-raw-auto-detect.vmx | 15 ++++++++++++ .../xml2vmx-cdrom-scsi-raw-auto-detect.xml | 14 +++++++++++ tests/xml2vmxtest.c | 2 ++ 11 files changed, 143 insertions(+), 5 deletions(-) create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.xml create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-auto-detect.vmx create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-auto-detect.xml create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-auto-detect.vmx create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-auto-detect.xml diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 40416a0..5c2c794 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2221,14 +2221,26 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con goto ignore; } else if (STRCASEEQ(deviceType, "atapi-cdrom")) { (*def)->type = VIR_DOMAIN_DISK_TYPE_BLOCK; - (*def)->src = fileName; - fileName = NULL; + + if (STRCASEEQ(fileName, "auto detect")) { + (*def)->src = NULL; + (*def)->startupPolicy = VIR_DOMAIN_STARTUP_POLICY_OPTIONAL; + } else { + (*def)->src = fileName; + fileName = NULL; + } } else if (STRCASEEQ(deviceType, "cdrom-raw")) { /* Raw access CD-ROMs actually are device='lun' */ (*def)->device = VIR_DOMAIN_DISK_DEVICE_LUN; (*def)->type = VIR_DOMAIN_DISK_TYPE_BLOCK; - (*def)->src = fileName; - fileName = NULL; + + if (STRCASEEQ(fileName, "auto detect")) { + (*def)->src = NULL; + (*def)->startupPolicy = VIR_DOMAIN_STARTUP_POLICY_OPTIONAL; + } else { + (*def)->src = fileName; + fileName = NULL; + } } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid or not yet handled value '%s' " @@ -3473,7 +3485,13 @@ virVMXFormatDisk(virVMXContext *ctx, virDomainDiskDefPtr def, VIR_FREE(fileName); } else if (def->type == VIR_DOMAIN_DISK_TYPE_BLOCK) { - if (def->src != NULL) { + if (!def->src && + def->startupPolicy == VIR_DOMAIN_STARTUP_POLICY_OPTIONAL) { + virBufferAsprintf(buffer, "%s%d:%d.autodetect = \"true\"\n", + busType, controllerOrBus, unit); + virBufferAsprintf(buffer, "%s%d:%d.fileName = \"auto detect\"\n", + busType, controllerOrBus, unit); + } else { virBufferAsprintf(buffer, "%s%d:%d.fileName = \"%s\"\n", busType, controllerOrBus, unit, def->src); } diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.vmx b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.vmx new file mode 100644 index 0000000..b2c4caf --- /dev/null +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.vmx @@ -0,0 +1,5 @@ +config.version = "8" +virtualHW.version = "4" +ide0:0.present = "true" +ide0:0.deviceType = "cdrom-raw" +ide0:0.fileName = "auto detect" diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml new file mode 100644 index 0000000..bce708d --- /dev/null +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml @@ -0,0 +1,24 @@ +<domain type='vmware'> + <uuid>00000000-0000-0000-0000-000000000000</uuid> + <memory unit='KiB'>32768</memory> + <currentMemory unit='KiB'>32768</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='block' device='lun'> + <source startupPolicy='optional'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <video> + <model type='vmvga' vram='4096'/> + </video> + </devices> +</domain> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.vmx b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.vmx new file mode 100644 index 0000000..0209b29 --- /dev/null +++ b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.vmx @@ -0,0 +1,6 @@ +config.version = "8" +virtualHW.version = "4" +scsi0.present = "true" +scsi0:0.present = "true" +scsi0:0.deviceType = "cdrom-raw" +scsi0:0.fileName = "auto detect" diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.xml b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.xml new file mode 100644 index 0000000..a81646a --- /dev/null +++ b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.xml @@ -0,0 +1,24 @@ +<domain type='vmware'> + <uuid>00000000-0000-0000-0000-000000000000</uuid> + <memory unit='KiB'>32768</memory> + <currentMemory unit='KiB'>32768</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='block' device='lun'> + <source startupPolicy='optional'/> + <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='scsi' index='0'/> + <video> + <model type='vmvga' vram='4096'/> + </video> + </devices> +</domain> diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c index 479c84c..5eb04e9 100644 --- a/tests/vmx2xmltest.c +++ b/tests/vmx2xmltest.c @@ -238,9 +238,11 @@ mymain(void) DO_TEST("cdrom-scsi-file", "cdrom-scsi-file"); DO_TEST("cdrom-scsi-device", "cdrom-scsi-device"); DO_TEST("cdrom-scsi-raw-device", "cdrom-scsi-raw-device"); + DO_TEST("cdrom-scsi-raw-auto-detect", "cdrom-scsi-raw-auto-detect"); DO_TEST("cdrom-ide-file", "cdrom-ide-file"); DO_TEST("cdrom-ide-device", "cdrom-ide-device"); DO_TEST("cdrom-ide-raw-device", "cdrom-ide-raw-device"); + DO_TEST("cdrom-ide-raw-auto-detect", "cdrom-ide-raw-auto-detect"); DO_TEST("floppy-file", "floppy-file"); DO_TEST("floppy-device", "floppy-device"); diff --git a/tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-auto-detect.vmx b/tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-auto-detect.vmx new file mode 100644 index 0000000..34e1467 --- /dev/null +++ b/tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-auto-detect.vmx @@ -0,0 +1,14 @@ +.encoding = "UTF-8" +config.version = "8" +virtualHW.version = "4" +guestOS = "other" +uuid.bios = "56 4d 9b ef ac d9 b4 e0-c8 f0 ae a8 b9 10 35 15" +displayName = "cdrom-ide-device" +memsize = "4" +numvcpus = "1" +ide0:0.present = "true" +ide0:0.deviceType = "cdrom-raw" +ide0:0.autodetect = "true" +ide0:0.fileName = "auto detect" +floppy0.present = "false" +floppy1.present = "false" diff --git a/tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-auto-detect.xml b/tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-auto-detect.xml new file mode 100644 index 0000000..166410c --- /dev/null +++ b/tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-auto-detect.xml @@ -0,0 +1,14 @@ +<domain type='vmware'> + <name>cdrom-ide-device</name> + <uuid>564d9bef-acd9-b4e0-c8f0-aea8b9103515</uuid> + <memory unit='KiB'>4096</memory> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='block' device='lun'> + <source startupPolicy='optional'/> + <target dev='hda' bus='ide'/> + </disk> + </devices> +</domain> diff --git a/tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-auto-detect.vmx b/tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-auto-detect.vmx new file mode 100644 index 0000000..84ec646 --- /dev/null +++ b/tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-auto-detect.vmx @@ -0,0 +1,15 @@ +.encoding = "UTF-8" +config.version = "8" +virtualHW.version = "4" +guestOS = "other" +uuid.bios = "56 4d 9b ef ac d9 b4 e0-c8 f0 ae a8 b9 10 35 15" +displayName = "cdrom-scsi-device" +memsize = "4" +numvcpus = "1" +scsi0.present = "true" +scsi0:0.present = "true" +scsi0:0.deviceType = "cdrom-raw" +scsi0:0.autodetect = "true" +scsi0:0.fileName = "auto detect" +floppy0.present = "false" +floppy1.present = "false" diff --git a/tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-auto-detect.xml b/tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-auto-detect.xml new file mode 100644 index 0000000..6531ec8 --- /dev/null +++ b/tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-auto-detect.xml @@ -0,0 +1,14 @@ +<domain type='vmware'> + <name>cdrom-scsi-device</name> + <uuid>564d9bef-acd9-b4e0-c8f0-aea8b9103515</uuid> + <memory unit='KiB'>4096</memory> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='block' device='lun'> + <source startupPolicy='optional'/> + <target dev='sda' bus='scsi'/> + </disk> + </devices> +</domain> diff --git a/tests/xml2vmxtest.c b/tests/xml2vmxtest.c index cb1c29c..cafcc36 100644 --- a/tests/xml2vmxtest.c +++ b/tests/xml2vmxtest.c @@ -254,9 +254,11 @@ mymain(void) DO_TEST("cdrom-scsi-file", "cdrom-scsi-file", 4); DO_TEST("cdrom-scsi-device", "cdrom-scsi-device", 4); DO_TEST("cdrom-scsi-raw-device", "cdrom-scsi-raw-device", 4); + DO_TEST("cdrom-scsi-raw-auto-detect", "cdrom-scsi-raw-auto-detect", 4); DO_TEST("cdrom-ide-file", "cdrom-ide-file", 4); DO_TEST("cdrom-ide-device", "cdrom-ide-device", 4); DO_TEST("cdrom-ide-raw-device", "cdrom-ide-raw-device", 4); + DO_TEST("cdrom-ide-raw-auto-detect", "cdrom-ide-raw-auto-detect", 4); DO_TEST("floppy-file", "floppy-file", 4); DO_TEST("floppy-device", "floppy-device", 4); -- 1.8.1.5
On 09/09/2013 07:48 PM, Doug Goldstein wrote:
VMWare Fusion 5 can set the CD-ROM's device name to be 'auto detect' when using the physical drive via 'cdrom-raw' device type. VMWare will then connect to first available host CD-ROM to the virtual machine upon start up according to VMWare documentation. If no device is available, it appears that the device will remain disconnected.
To better model this a CD-ROM that is marked as "auto detect" when in the off state would be modeled as the following with this patch: <disk type='block' device='lun'> <source startupPolicy='optional'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk>
Once the domain transitions to the powered on state, libvirt can populate the remaining source data with what is connected, if anything. However future power cycles, the domain may not always start with that device attached. ---
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
A user was having an issue with this specific VMWare Fusion config and he gave me permission to add it as part of our test suite to further expand our VMX test coverage. Unfortunately our VMX parser and generator does not support many features contained within and just silently ignores fields it does not understand so they had to be removed out in the xml2vmx test. The original unmodified version exists in the vmx2xml test. --- tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx | 88 ++++++++++++++++++++++ tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml | 39 ++++++++++ tests/vmx2xmltest.c | 2 + tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.vmx | 30 ++++++++ tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml | 41 ++++++++++ tests/xml2vmxtest.c | 2 + 6 files changed, 202 insertions(+) create mode 100644 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml create mode 100644 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.vmx create mode 100644 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml diff --git a/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx b/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx new file mode 100644 index 0000000..ef6af19 --- /dev/null +++ b/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx @@ -0,0 +1,88 @@ +.encoding = "UTF-8" +config.version = "8" +virtualHW.version = "9" +memsize = "3572" +MemAllowAutoScaleDown = "FALSE" +MemTrimRate = "-1" +displayName = "ATTM_VM" +guestOS = "winxppro" +numvcpus = "2" +sound.present = "TRUE" +sound.filename = "-1" +sound.autodetect = "TRUE" +usb.present = "TRUE" +ethernet0.present = "TRUE" +ethernet0.addressType = "generated" +ethernet0.connectionType = "bridged" +ethernet1.present = "TRUE" +ethernet1.addressType = "generated" +ethernet1.connectionType = "bridged" +scsi0:0.present = "TRUE" +scsi0:0.fileName = "ATTM_VM.vmdk" +pciBridge0.present = "TRUE" +tools.upgrade.policy = "useGlobal" +ehci.present = "TRUE" +ide0:0.present = "TRUE" +ide0:0.autodetect = "TRUE" +ide0:0.filename = "auto detect" +ide0:0.deviceType = "atapi-cdrom" +scsi0.present = "TRUE" +scsi0.virtualDev = "buslogic" +buslogic.noDriver = "FALSE" +extendedConfigFile = "ATTM_VM.vmxf" +virtualHW.productCompatibility = "hosted" +pciBridge4.present = "TRUE" +pciBridge4.virtualDev = "pcieRootPort" +pciBridge4.pciSlotNumber = "21" +pciBridge4.functions = "8" +pciBridge5.present = "TRUE" +pciBridge5.virtualDev = "pcieRootPort" +pciBridge5.pciSlotNumber = "22" +pciBridge5.functions = "8" +pciBridge6.present = "TRUE" +pciBridge6.virtualDev = "pcieRootPort" +pciBridge6.pciSlotNumber = "23" +pciBridge6.functions = "8" +pciBridge7.present = "TRUE" +pciBridge7.virtualDev = "pcieRootPort" +pciBridge7.pciSlotNumber = "24" +pciBridge7.functions = "8" +vmci0.present = "TRUE" +hpet0.present = "TRUE" +usb.vbluetooth.startConnected = "TRUE" +mks.enable3d = "TRUE" +ethernet0.linkStatePropagation.enable = "TRUE" +ethernet1.linkStatePropagation.enable = "TRUE" +ide0:0.startConnected = "FALSE" +ethernet0.generatedAddress = "00:0c:29:3b:64:ea" +ethernet1.generatedAddress = "00:0c:29:3b:64:f4" +vmci0.id = "-952408854" +tools.syncTime = "FALSE" +uuid.location = "56 4d 70 88 01 a1 98 32-e7 2b 67 90 c7 3b 64 ea" +uuid.bios = "56 4d 70 88 01 a1 98 32-e7 2b 67 90 c7 3b 64 ea" +cleanShutdown = "TRUE" +replay.supported = "FALSE" +replay.filename = "" +scsi0:0.redo = "" +pciBridge0.pciSlotNumber = "17" +scsi0.pciSlotNumber = "16" +usb.pciSlotNumber = "32" +ethernet0.pciSlotNumber = "33" +ethernet1.pciSlotNumber = "34" +sound.pciSlotNumber = "35" +ehci.pciSlotNumber = "36" +vmci0.pciSlotNumber = "37" +usb:1.present = "TRUE" +ethernet0.generatedAddressOffset = "0" +ethernet1.generatedAddressOffset = "10" +vmotion.checkpointFBSize = "134217728" +usb:1.speed = "2" +usb:1.deviceType = "hub" +usb:1.port = "1" +usb:1.parent = "-1" +floppy0.startConnected = "FALSE" +softPowerOff = "FALSE" +usb:0.present = "TRUE" +usb:0.deviceType = "hid" +usb:0.port = "0" +usb:0.parent = "-1" diff --git a/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml b/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml new file mode 100644 index 0000000..ef785d7 --- /dev/null +++ b/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml @@ -0,0 +1,39 @@ +<domain type='vmware'> + <name>ATTM_VM</name> + <uuid>564d7088-01a1-9832-e72b-6790c73b64ea</uuid> + <memory unit='KiB'>3657728</memory> + <currentMemory unit='KiB'>3657728</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='i686'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='file' device='disk'> + <source file='[datastore] directory/ATTM_VM.vmdk'/> + <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='block' device='cdrom'> + <source startupPolicy='optional'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='scsi' index='0' model='buslogic'/> + <controller type='ide' index='0'/> + <interface type='bridge'> + <mac address='00:0c:29:3b:64:ea'/> + <source bridge=''/> + </interface> + <interface type='bridge'> + <mac address='00:0c:29:3b:64:f4'/> + <source bridge=''/> + </interface> + <video> + <model type='vmvga' vram='4096'/> + </video> + </devices> +</domain> diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c index 5eb04e9..00bf21d 100644 --- a/tests/vmx2xmltest.c +++ b/tests/vmx2xmltest.c @@ -288,6 +288,8 @@ mymain(void) DO_TEST("ws-in-the-wild-1", "ws-in-the-wild-1"); DO_TEST("ws-in-the-wild-2", "ws-in-the-wild-2"); + DO_TEST("fusion-in-the-wild-1", "fusion-in-the-wild-1"); + DO_TEST("annotation", "annotation"); DO_TEST("smbios", "smbios"); diff --git a/tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.vmx b/tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.vmx new file mode 100644 index 0000000..5693210 --- /dev/null +++ b/tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.vmx @@ -0,0 +1,30 @@ +.encoding = "UTF-8" +config.version = "8" +virtualHW.version = "9" +guestOS = "other" +uuid.bios = "56 4d 70 88 01 a1 98 32-e7 2b 67 90 c7 3b 64 ea" +displayName = "ATTM_VM" +memsize = "3572" +numvcpus = "2" +scsi0.present = "true" +scsi0.virtualDev = "buslogic" +scsi0:0.present = "true" +scsi0:0.deviceType = "scsi-hardDisk" +scsi0:0.fileName = "/vmfs/volumes/datastore/directory/ATTM_VM.vmdk" +ide0:0.present = "true" +ide0:0.deviceType = "atapi-cdrom" +ide0:0.autodetect = "true" +ide0:0.fileName = "auto detect" +floppy0.present = "false" +floppy1.present = "false" +ethernet0.present = "true" +ethernet0.connectionType = "bridged" +ethernet0.addressType = "generated" +ethernet0.generatedAddress = "00:0c:29:3b:64:ea" +ethernet0.generatedAddressOffset = "0" +ethernet1.present = "true" +ethernet1.connectionType = "bridged" +ethernet1.addressType = "generated" +ethernet1.generatedAddress = "00:0c:29:3b:64:f4" +ethernet1.generatedAddressOffset = "0" +svga.vramSize = "4194304" diff --git a/tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml b/tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml new file mode 100644 index 0000000..0bfc7e8 --- /dev/null +++ b/tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml @@ -0,0 +1,41 @@ +<domain type='vmware'> + <name>ATTM_VM</name> + <uuid>564d7088-01a1-9832-e72b-6790c73b64ea</uuid> + <memory unit='KiB'>3657728</memory> + <currentMemory unit='KiB'>3657728</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='i686'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='file' device='disk'> + <source file='[datastore] directory/ATTM_VM.vmdk'/> + <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='block' device='cdrom'> + <driver name='atapi'/> + <source startupPolicy='optional'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='scsi' index='0' model='buslogic'/> + <controller type='ide' index='0'/> + <interface type='bridge'> + <mac address='00:0c:29:3b:64:ea'/> + <source bridge=''/> + </interface> + <interface type='bridge'> + <mac address='00:0c:29:3b:64:f4'/> + <source bridge=''/> + </interface> + <video> + <model type='vmvga' vram='4096'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/xml2vmxtest.c b/tests/xml2vmxtest.c index cafcc36..a8adc82 100644 --- a/tests/xml2vmxtest.c +++ b/tests/xml2vmxtest.c @@ -301,6 +301,8 @@ mymain(void) DO_TEST("ws-in-the-wild-1", "ws-in-the-wild-1", 8); DO_TEST("ws-in-the-wild-2", "ws-in-the-wild-2", 8); + DO_TEST("fusion-in-the-wild-1", "fusion-in-the-wild-1", 9); + DO_TEST("annotation", "annotation", 4); DO_TEST("smbios", "smbios", 4); -- 1.8.1.5
On 09/09/2013 07:48 PM, Doug Goldstein wrote:
A user was having an issue with this specific VMWare Fusion config and he gave me permission to add it as part of our test suite to further expand our VMX test coverage. Unfortunately our VMX parser and generator does not support many features contained within and just silently ignores fields it does not understand so they had to be removed out in the xml2vmx test. The original unmodified version exists in the vmx2xml test. --- tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx | 88 ++++++++++++++++++++++ tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml | 39 ++++++++++ tests/vmx2xmltest.c | 2 + tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.vmx | 30 ++++++++ tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml | 41 ++++++++++ tests/xml2vmxtest.c | 2 + 6 files changed, 202 insertions(+) create mode 100644 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml create mode 100644 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.vmx create mode 100644 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml
ACK. More coverage never hurts :) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Sep 9, 2013 at 8:48 PM, Doug Goldstein <cardoe@cardoe.com> wrote:
A user came into #virt the other day and was trying to get libvirtd to work with VMware Fusion 5, which is basically the Mac OS X version of VMware Workstation. In helping him out I noticed a few limitations of our VMX parser so I've added support through this patchset. I don't personally have access or own VMware's product.
v4: * Half the patchset has been merged at this point * Add back 'auto detect' support via <source startupPolicy='optional'/> * Made device='lun' match device='cdrom' * Patch 1 & 2 are new to the series
v3: * Dropped 'auto detect' support from series as it needs more work based on feedback * Added patch to combine virVMXFormatHardDisk and virVMXFormatCDROM into one function. * Converted to <disk type='block' device='lun'> instead of adding a <driver> element to better match the behavior available via QEMU.
v2: * Added additional test cases and fixed issues that arose from those
*** BLURB HERE ***
Doug Goldstein (4): Allow LUN type disks to have no source Allow <source> for type=block to have no dev VMX: Add support for 'auto detect' fileNames VMX: Add a VMWare Fusion 5 configuration for tests
docs/schemas/domaincommon.rng | 8 +- src/conf/domain_conf.c | 9 ++- src/vmx/vmx.c | 28 +++++-- .../vmx2xml-cdrom-ide-raw-auto-detect.vmx | 5 ++ .../vmx2xml-cdrom-ide-raw-auto-detect.xml | 24 ++++++ .../vmx2xml-cdrom-scsi-raw-auto-detect.vmx | 6 ++ .../vmx2xml-cdrom-scsi-raw-auto-detect.xml | 24 ++++++ tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx | 88 ++++++++++++++++++++++ tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml | 39 ++++++++++ tests/vmx2xmltest.c | 4 + .../xml2vmx-cdrom-ide-raw-auto-detect.vmx | 14 ++++ .../xml2vmx-cdrom-ide-raw-auto-detect.xml | 14 ++++ .../xml2vmx-cdrom-scsi-raw-auto-detect.vmx | 15 ++++ .../xml2vmx-cdrom-scsi-raw-auto-detect.xml | 14 ++++ tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.vmx | 30 ++++++++ tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml | 41 ++++++++++ tests/xml2vmxtest.c | 4 + 17 files changed, 356 insertions(+), 11 deletions(-) create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.xml create mode 100644 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-auto-detect.vmx create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-auto-detect.xml create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-auto-detect.vmx create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-auto-detect.xml create mode 100644 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.vmx create mode 100644 tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml
-- 1.8.1.5
Ping for anyone with free time to review. -- Doug Goldstein
participants (2)
-
Doug Goldstein -
Eric Blake