From 7fd73c056fa74ab32f5976ccbd2ce516f504088a Mon Sep 17 00:00:00 2001 Message-Id: <7fd73c056fa74ab32f5976ccbd2ce516f504088a@dist-git> From: Pino Toscano Date: Mon, 23 Mar 2020 08:49:58 +0100 Subject: [PATCH] vmx: make 'fileName' optional for CD-ROMs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It seems like CD-ROMs may have no 'fileName' property specified in case there is nothing configured as attachment for the drive. Hence, make sure that virVMXParseDisk() do not consider it mandatory anymore, considering it an empty block cdrom device. Sadly virVMXParseDisk() is used also to parse disk and floppies, so make sure that a NULL fileName is handled in cdrom- and floppy-related paths. https://bugzilla.redhat.com/show_bug.cgi?id=1808610 Signed-off-by: Pino Toscano Reviewed-by: Ján Tomko Tested-by: Richard W.M. Jones (cherry picked from commit c5ee737bc5c0fc61451e45ff41526270bdf0113c) https://bugzilla.redhat.com/show_bug.cgi?id=1815269 https://bugzilla.redhat.com/show_bug.cgi?id=1816035 https://bugzilla.redhat.com/show_bug.cgi?id=1816034 Signed-off-by: Pino Toscano Message-Id: <20200323074958.377314-3-ptoscano@redhat.com> Reviewed-by: Ján Tomko --- src/vmx/vmx.c | 25 ++++++++++--------- .../vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx | 4 +++ .../vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml | 23 +++++++++++++++++ tests/vmx2xmltest.c | 1 + 4 files changed, 41 insertions(+), 12 deletions(-) create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 89409870ff..14e5133ef7 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2204,7 +2204,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con goto cleanup; /* vmx:fileName -> def:src, def:type */ - if (virVMXGetConfigString(conf, fileName_name, &fileName, false) < 0) + if (virVMXGetConfigString(conf, fileName_name, &fileName, true) < 0) goto cleanup; /* vmx:writeThrough -> def:cachemode */ @@ -2215,7 +2215,8 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con /* Setup virDomainDiskDef */ if (device == VIR_DOMAIN_DISK_DEVICE_DISK) { - if (virFileHasSuffix(fileName, ".iso") || + if (fileName == NULL || + virFileHasSuffix(fileName, ".iso") || STREQ(fileName, "emptyBackingString") || (deviceType && (STRCASEEQ(deviceType, "atapi-cdrom") || @@ -2274,7 +2275,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con goto cleanup; } } else if (device == VIR_DOMAIN_DISK_DEVICE_CDROM) { - if (virFileHasSuffix(fileName, ".vmdk")) { + if (fileName && virFileHasSuffix(fileName, ".vmdk")) { /* * This function was called in order to parse a CDROM device, but * .vmdk files are for harddisk devices only. Just ignore it, @@ -2282,7 +2283,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con * handle it. */ goto ignore; - } else if (virFileHasSuffix(fileName, ".iso")) { + } else if (fileName && virFileHasSuffix(fileName, ".iso")) { char *tmp; if (deviceType && STRCASENEQ(deviceType, "cdrom-image")) { @@ -2303,7 +2304,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con } else if (deviceType && STRCASEEQ(deviceType, "atapi-cdrom")) { virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK); - if (STRCASEEQ(fileName, "auto detect")) { + if (fileName && STRCASEEQ(fileName, "auto detect")) { ignore_value(virDomainDiskSetSource(*def, NULL)); (*def)->startupPolicy = VIR_DOMAIN_STARTUP_POLICY_OPTIONAL; } else if (virDomainDiskSetSource(*def, fileName) < 0) { @@ -2314,7 +2315,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con (*def)->device = VIR_DOMAIN_DISK_DEVICE_LUN; virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK); - if (STRCASEEQ(fileName, "auto detect")) { + if (fileName && STRCASEEQ(fileName, "auto detect")) { ignore_value(virDomainDiskSetSource(*def, NULL)); (*def)->startupPolicy = VIR_DOMAIN_STARTUP_POLICY_OPTIONAL; } else if (virDomainDiskSetSource(*def, fileName) < 0) { @@ -2322,7 +2323,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con } } else if (busType == VIR_DOMAIN_DISK_BUS_SCSI && deviceType && STRCASEEQ(deviceType, "scsi-passthru")) { - if (STRPREFIX(fileName, "/vmfs/devices/cdrom/")) { + if (fileName && STRPREFIX(fileName, "/vmfs/devices/cdrom/")) { /* SCSI-passthru CD-ROMs actually are device='lun' */ (*def)->device = VIR_DOMAIN_DISK_DEVICE_LUN; virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK); @@ -2338,7 +2339,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con */ goto ignore; } - } else if (STREQ(fileName, "emptyBackingString")) { + } else if (fileName && STREQ(fileName, "emptyBackingString")) { if (deviceType && STRCASENEQ(deviceType, "cdrom-image")) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Expecting VMX entry '%s' to be 'cdrom-image' " @@ -2352,7 +2353,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid or not yet handled value '%s' " "for VMX entry '%s' for device type '%s'"), - fileName, fileName_name, + NULLSTR(fileName), fileName_name, deviceType ? deviceType : "unknown"); goto cleanup; } @@ -2362,10 +2363,10 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con if (virDomainDiskSetSource(*def, fileName) < 0) goto cleanup; } else if (fileType != NULL && STRCASEEQ(fileType, "file")) { - char *tmp; + char *tmp = NULL; virDomainDiskSetType(*def, VIR_STORAGE_TYPE_FILE); - if (!(tmp = ctx->parseFileName(fileName, ctx->opaque))) + if (fileName && !(tmp = ctx->parseFileName(fileName, ctx->opaque))) goto cleanup; if (virDomainDiskSetSource(*def, tmp) < 0) { VIR_FREE(tmp); @@ -2376,7 +2377,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid or not yet handled value '%s' " "for VMX entry '%s' for device type '%s'"), - fileName, fileName_name, + NULLSTR(fileName), fileName_name, deviceType ? deviceType : "unknown"); goto cleanup; } diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx new file mode 100644 index 0000000000..36286cb20f --- /dev/null +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx @@ -0,0 +1,4 @@ +config.version = "8" +virtualHW.version = "4" +ide0:0.present = "true" +ide0:0.deviceType = "atapi-cdrom" diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml new file mode 100644 index 0000000000..af4a5ff9f6 --- /dev/null +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml @@ -0,0 +1,23 @@ + + 00000000-0000-0000-0000-000000000000 + 32768 + 32768 + 1 + + hvm + + + destroy + restart + destroy + + + +
+ + + + + diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c index 7289dc91e3..143ff4c937 100644 --- a/tests/vmx2xmltest.c +++ b/tests/vmx2xmltest.c @@ -227,6 +227,7 @@ mymain(void) DO_TEST("cdrom-scsi-passthru", "cdrom-scsi-passthru"); DO_TEST("cdrom-ide-file", "cdrom-ide-file"); DO_TEST("cdrom-ide-empty", "cdrom-ide-empty"); + DO_TEST("cdrom-ide-empty-2", "cdrom-ide-empty-2"); 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"); -- 2.26.0