[libvirt] [PATCHv5 0/2] Rework of Disk Geometry Support
With qemu it is possible to override the geometry (cylinders, heads, sectors) of disks. This series adds a new XML tag for geometry and the related support in the qemu driver. V2 Changes: Refactor into the generic domain configuration/XML support for disk geometry and a qemu specific part with the testcase V3 Changes: 1. Use virReportError instead of custom xxxReportError. 2. Moved the geometry element to diskspec, so it applies to all kind of disks. V4 Changes: Rebase for libvirt 0.10.0 freeze. V5 Changes: Corrected qemuxml2argvtest invocation. Oversight after rebase. J.B. Joret (2): Support for Disk Geometry Override qemu: Disk Geometry Override Support docs/formatdomain.html.in | 25 +++++++ docs/schemas/domaincommon.rng | 25 +++++++ src/conf/domain_conf.c | 71 ++++++++++++++++++++ src/conf/domain_conf.h | 17 +++++ src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 59 ++++++++++++++++ .../qemuxml2argv-disk-geometry.args | 4 + .../qemuxml2argv-disk-geometry.xml | 26 +++++++ tests/qemuxml2argvtest.c | 2 + 9 files changed, 231 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.xml
From: J.B. Joret <jb@linux.vnet.ibm.com> A hypervisor may allow to override the disk geometry of drives. Qemu, as an example with cyls=,heads=,secs=[,trans=]. This patch extends the domain config to allow the specification of disk geometry with libvirt. V2 Changes: Split out qemu specific code, add documentation and schema. V3 Changes: Moved geometry element to diskspec and use virReportError now. Signed-off-by: J.B. Joret <jb@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 25 ++++++++++++++ docs/schemas/domaincommon.rng | 25 ++++++++++++++ src/conf/domain_conf.c | 71 +++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 17 ++++++++++ src/libvirt_private.syms | 2 + 5 files changed, 140 insertions(+), 0 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 2c5c456..bbf69bb 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1224,6 +1224,12 @@ <target dev='sda' bus='scsi'/> <address type='drive' controller='0' bus='0' target='3' unit='0'/> </disk> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/sda'/> + <geometry cyls='16383' heads='16' secs='63' trans='lba'/> + <target dev='hda' bus='ide'/> + </disk> </devices> ...</pre> @@ -1571,6 +1577,25 @@ associated with the Ceph secret object. <span class="since">libvirt 0.9.7</span> </dd> + <dt><code>geometry</code></dt> + <dd>The optional <code>geometry</code> element provides the + ability to override geometry settings. This mostly useful for + S390 DASD-disks or older DOS-disks + <dl> + <dt><code>cyls</code></dt> + <dd>The <code>cyls</code> element is the + number of cylinders. </dd> + <dt><code>heads</code></dt> + <dd>The <code>heads</code> element is the + number of heads. </dd> + <dt><code>secs</code></dt> + <dd>The <code>secs</code> element is the + number of sectors per track. </dd> + <dt><code>trans</code></dt> + <dd>The optional <code>trans</code> element is the + BIOS-Translation-Modus (none, lba or auto)</dd> + </dl> + </dd> </dl> <h4><a name="elementsFilesystems">Filesystems</a></h4> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4903ca6..428df54 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -852,6 +852,9 @@ <optional> <ref name="address"/> </optional> + <optional> + <ref name="geometry"/> + </optional> </interleave> </define> <define name="snapshot"> @@ -1054,6 +1057,28 @@ </optional> </element> </define> + <define name="geometry"> + <element name="geometry"> + <attribute name="cyls"> + <data type="integer"/> + </attribute> + <attribute name="heads"> + <data type="integer"/> + </attribute> + <attribute name="secs"> + <data type="integer"/> + </attribute> + <optional> + <attribute name="trans"> + <choice> + <value>auto</value> + <value>none</value> + <value>lba</value> + </choice> + </attribute> + </optional> + </element> + </define> <!-- Disk may use a special driver for access. --> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a2e73c6..7662a6a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -171,6 +171,12 @@ VIR_ENUM_IMPL(virDomainDiskDevice, VIR_DOMAIN_DISK_DEVICE_LAST, "floppy", "lun") +VIR_ENUM_IMPL(virDomainDiskGeometryTrans, VIR_DOMAIN_DISK_TRANS_LAST, + "default", + "none", + "auto", + "lba") + VIR_ENUM_IMPL(virDomainDiskBus, VIR_DOMAIN_DISK_BUS_LAST, "ide", "fdc", @@ -3336,6 +3342,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, char *source = NULL; char *target = NULL; char *protocol = NULL; + char *trans = NULL; virDomainDiskHostDefPtr hosts = NULL; int nhosts = 0; char *bus = NULL; @@ -3364,6 +3371,11 @@ virDomainDiskDefParseXML(virCapsPtr caps, return NULL; } + def->geometry.cylinders = 0; + def->geometry.heads = 0; + def->geometry.sectors = 0; + def->geometry.trans = VIR_DOMAIN_DISK_TRANS_DEFAULT; + ctxt->node = node; type = virXMLPropString(node, "type"); @@ -3473,6 +3485,40 @@ virDomainDiskDefParseXML(virCapsPtr caps, if (target && STRPREFIX(target, "ioemu:")) memmove(target, target+6, strlen(target)-5); + } else if (xmlStrEqual(cur->name, BAD_CAST "geometry")) { + if (virXPathUInt("string(./geometry/@cyls)", + ctxt, &def->geometry.cylinders) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid geometry settings (cyls)")); + goto error; + } + if (virXPathUInt("string(./geometry/@heads)", + ctxt, &def->geometry.heads) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid geometry settings (heads)")); + goto error; + } + if (virXPathUInt("string(./geometry/@secs)", + ctxt, &def->geometry.sectors) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid geometry settings (secs)")); + goto error; + } + trans = virXMLPropString(cur, "trans"); + if (trans != NULL) { + if (STREQ(trans, "none")) + def->geometry.trans = VIR_DOMAIN_DISK_TRANS_NONE; + else if (STREQ(trans, "auto")) + def->geometry.trans = VIR_DOMAIN_DISK_TRANS_AUTO; + else if (STREQ(trans, "lba")) + def->geometry.trans = VIR_DOMAIN_DISK_TRANS_LBA; + else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid translation value '%s'"), + trans); + goto error; + } + } } else if (!driverName && xmlStrEqual(cur->name, BAD_CAST "driver")) { driverName = virXMLPropString(cur, "name"); @@ -3951,6 +3997,7 @@ cleanup: VIR_FREE(target); VIR_FREE(source); VIR_FREE(tray); + VIR_FREE(trans); while (nhosts > 0) { virDomainDiskHostDefFree(&hosts[nhosts - 1]); nhosts--; @@ -11081,6 +11128,28 @@ virDomainLeaseDefFormat(virBufferPtr buf, return 0; } +static void virDomainDiskGeometryDefFormat(virBufferPtr buf, + virDomainDiskDefPtr def) +{ + const char *trans = + virDomainDiskGeometryTransTypeToString(def->geometry.trans); + + if (def->geometry.cylinders > 0 && + def->geometry.heads > 0 && + def->geometry.sectors > 0) { + virBufferAsprintf(buf, + " <geometry cyls='%u' heads='%u' secs='%u'", + def->geometry.cylinders, + def->geometry.heads, + def->geometry.sectors); + + if (def->geometry.trans != VIR_DOMAIN_DISK_TRANS_DEFAULT) + virBufferEscapeString(buf, " trans='%s'", trans); + + virBufferAddLit(buf, "/>\n"); + } +} + static int virDomainDiskDefFormat(virBufferPtr buf, virDomainDiskDefPtr def, @@ -11202,6 +11271,7 @@ virDomainDiskDefFormat(virBufferPtr buf, } else { virBufferAddLit(buf, "/>\n"); } + virDomainDiskGeometryDefFormat(buf, def); break; case VIR_DOMAIN_DISK_TYPE_BLOCK: virBufferEscapeString(buf, " <source dev='%s'", @@ -11215,6 +11285,7 @@ virDomainDiskDefFormat(virBufferPtr buf, } else { virBufferAddLit(buf, "/>\n"); } + virDomainDiskGeometryDefFormat(buf, def); break; case VIR_DOMAIN_DISK_TYPE_DIR: virBufferEscapeString(buf, " <source dir='%s'/>\n", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index fae7792..3b62e4c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -469,6 +469,15 @@ enum virDomainDiskTray { VIR_DOMAIN_DISK_TRAY_LAST }; +enum virDomainDiskGeometryTrans { + VIR_DOMAIN_DISK_TRANS_DEFAULT = 0, + VIR_DOMAIN_DISK_TRANS_NONE, + VIR_DOMAIN_DISK_TRANS_AUTO, + VIR_DOMAIN_DISK_TRANS_LBA, + + VIR_DOMAIN_DISK_TRANS_LAST +}; + typedef struct _virDomainDiskHostDef virDomainDiskHostDef; typedef virDomainDiskHostDef *virDomainDiskHostDefPtr; struct _virDomainDiskHostDef { @@ -578,6 +587,13 @@ struct _virDomainDiskDef { char *mirrorFormat; bool mirroring; + struct { + unsigned int cylinders; + unsigned int heads; + unsigned int sectors; + int trans; + } geometry; + virDomainBlockIoTuneInfo blkdeviotune; char *serial; @@ -2171,6 +2187,7 @@ VIR_ENUM_DECL(virDomainDeviceAddress) VIR_ENUM_DECL(virDomainDeviceAddressPciMulti) VIR_ENUM_DECL(virDomainDisk) VIR_ENUM_DECL(virDomainDiskDevice) +VIR_ENUM_DECL(virDomainDiskGeometryTrans) VIR_ENUM_DECL(virDomainDiskBus) VIR_ENUM_DECL(virDomainDiskCache) VIR_ENUM_DECL(virDomainDiskErrorPolicy) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e91540d..7f4c1c6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -315,6 +315,8 @@ virDomainDiskDefAssignAddress; virDomainDiskDefForeachPath; virDomainDiskDefFree; virDomainDiskDeviceTypeToString; +virDomainDiskGeometryTransTypeToString; +virDomainDiskGeometryTransTypeFromString; virDomainDiskErrorPolicyTypeFromString; virDomainDiskErrorPolicyTypeToString; virDomainDiskFindControllerModel; -- 1.7.0.4
On Mon, Aug 20, 2012 at 01:58:24PM +0200, Viktor Mihajlovski wrote:
From: J.B. Joret <jb@linux.vnet.ibm.com>
A hypervisor may allow to override the disk geometry of drives. Qemu, as an example with cyls=,heads=,secs=[,trans=]. This patch extends the domain config to allow the specification of disk geometry with libvirt.
V2 Changes: Split out qemu specific code, add documentation and schema.
V3 Changes: Moved geometry element to diskspec and use virReportError now.
Signed-off-by: J.B. Joret <jb@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 25 ++++++++++++++ docs/schemas/domaincommon.rng | 25 ++++++++++++++ src/conf/domain_conf.c | 71 +++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 17 ++++++++++ src/libvirt_private.syms | 2 + 5 files changed, 140 insertions(+), 0 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
From: J.B. Joret <jb@linux.vnet.ibm.com> Qemu command line generation for geometry override and testcases. V2 Changes: squashed qemu code and testcases. V3 Changes: use virReportError. V4 Changes: rebase V5 Changes: Fixed test invocation for geometry. Signed-off-by: J.B. Joret <jb@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 59 ++++++++++++++++++++ .../qemuxml2argv-disk-geometry.args | 4 + .../qemuxml2argv-disk-geometry.xml | 26 +++++++++ tests/qemuxml2argvtest.c | 2 + 4 files changed, 91 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9383530..cc44015 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2073,6 +2073,8 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, { virBuffer opt = VIR_BUFFER_INITIALIZER; const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); + const char *trans = + virDomainDiskGeometryTransTypeToString(disk->geometry.trans); int idx = virDiskNameToIndex(disk->dst); int busid = -1, unitid = -1; @@ -2275,6 +2277,24 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, disk->type != VIR_DOMAIN_DISK_TYPE_DIR && qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT)) virBufferAsprintf(&opt, ",format=%s", disk->driverType); + + /* generate geometry command string*/ + if (disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK || + disk->type == VIR_DOMAIN_DISK_TYPE_FILE) { + if (disk->geometry.cylinders > 0 && + disk->geometry.heads > 0 && + disk->geometry.sectors > 0) { + + virBufferAsprintf(&opt, ",cyls=%u,heads=%u,secs=%u", + disk->geometry.cylinders, + disk->geometry.heads, + disk->geometry.sectors); + + if (disk->geometry.trans != VIR_DOMAIN_DISK_TRANS_DEFAULT) + virBufferEscapeString(&opt, ",trans=%s", trans); + } + } + if (disk->serial && qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE_SERIAL)) { if (qemuSafeSerialParamValue(disk->serial) < 0) @@ -6691,6 +6711,7 @@ qemuParseCommandLineDisk(virCapsPtr caps, int idx = -1; int busid = -1; int unitid = -1; + int trans = VIR_DOMAIN_DISK_TRANS_DEFAULT; if ((nkeywords = qemuParseKeywords(val, &keywords, @@ -6895,6 +6916,44 @@ qemuParseCommandLineDisk(virCapsPtr caps, virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse io mode '%s'"), values[i]); } + } else if (STREQ(keywords[i], "cyls")) { + if (virStrToLong_ui(values[i], NULL, 10, + &(def->geometry.cylinders)) < 0) { + virDomainDiskDefFree(def); + def = NULL; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse cylinders value'%s'"), + values[i]); + } + } else if (STREQ(keywords[i], "heads")) { + if (virStrToLong_ui(values[i], NULL, 10, + &(def->geometry.heads)) < 0) { + virDomainDiskDefFree(def); + def = NULL; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse heads value'%s'"), + values[i]); + } + } else if (STREQ(keywords[i], "secs")) { + if (virStrToLong_ui(values[i], NULL, 10, + &(def->geometry.sectors)) < 0) { + virDomainDiskDefFree(def); + def = NULL; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse sectors value'%s'"), + values[i]); + } + } else if (STREQ(keywords[i], "trans")) { + def->geometry.trans = + virDomainDiskGeometryTransTypeFromString(values[i]); + if ((trans < VIR_DOMAIN_DISK_TRANS_DEFAULT) || + (trans >= VIR_DOMAIN_DISK_TRANS_LAST)) { + virDomainDiskDefFree(def); + def = NULL; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse translation value'%s'"), + values[i]); + } } } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args new file mode 100644 index 0000000..7cd6650 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args @@ -0,0 +1,4 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0,cyls=16383,heads=16,secs=63,trans=lba \ +-net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.xml new file mode 100644 index 0000000..f0cf4ae --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.xml @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219136</memory> + <currentMemory>219136</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <geometry cyls='16383' heads='16' secs='63' trans='lba'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index f8d8db5..71513fb 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -788,6 +788,8 @@ mymain(void) QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD); + DO_TEST("disk-geometry", QEMU_CAPS_DRIVE); + VIR_FREE(driver.stateDir); virCapabilitiesFree(driver.caps); VIR_FREE(map); -- 1.7.0.4
On Mon, Aug 20, 2012 at 01:58:25PM +0200, Viktor Mihajlovski wrote:
From: J.B. Joret <jb@linux.vnet.ibm.com>
Qemu command line generation for geometry override and testcases.
V2 Changes: squashed qemu code and testcases.
V3 Changes: use virReportError.
V4 Changes: rebase
V5 Changes: Fixed test invocation for geometry.
Signed-off-by: J.B. Joret <jb@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 59 ++++++++++++++++++++ .../qemuxml2argv-disk-geometry.args | 4 + .../qemuxml2argv-disk-geometry.xml | 26 +++++++++ tests/qemuxml2argvtest.c | 2 + 4 files changed, 91 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9383530..cc44015 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2073,6 +2073,8 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, { virBuffer opt = VIR_BUFFER_INITIALIZER; const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); + const char *trans = + virDomainDiskGeometryTransTypeToString(disk->geometry.trans); int idx = virDiskNameToIndex(disk->dst); int busid = -1, unitid = -1;
@@ -2275,6 +2277,24 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, disk->type != VIR_DOMAIN_DISK_TYPE_DIR && qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT)) virBufferAsprintf(&opt, ",format=%s", disk->driverType); + + /* generate geometry command string*/ + if (disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK || + disk->type == VIR_DOMAIN_DISK_TYPE_FILE) {
The cylinders/heads/sectors data is part of the guest side device model. The disk->type == block|file|network is part of the host side device model. IIUC, there should be no dependency between host and guest side data for this property, so IMHO this if() should be removed.
+ if (disk->geometry.cylinders > 0 && + disk->geometry.heads > 0 && + disk->geometry.sectors > 0) { + + virBufferAsprintf(&opt, ",cyls=%u,heads=%u,secs=%u", + disk->geometry.cylinders, + disk->geometry.heads, + disk->geometry.sectors); + + if (disk->geometry.trans != VIR_DOMAIN_DISK_TRANS_DEFAULT) + virBufferEscapeString(&opt, ",trans=%s", trans); + } + } + if (disk->serial && qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE_SERIAL)) { if (qemuSafeSerialParamValue(disk->serial) < 0) @@ -6691,6 +6711,7 @@ qemuParseCommandLineDisk(virCapsPtr caps, int idx = -1; int busid = -1; int unitid = -1; + int trans = VIR_DOMAIN_DISK_TRANS_DEFAULT;
if ((nkeywords = qemuParseKeywords(val, &keywords, @@ -6895,6 +6916,44 @@ qemuParseCommandLineDisk(virCapsPtr caps, virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse io mode '%s'"), values[i]); } + } else if (STREQ(keywords[i], "cyls")) { + if (virStrToLong_ui(values[i], NULL, 10, + &(def->geometry.cylinders)) < 0) { + virDomainDiskDefFree(def); + def = NULL; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse cylinders value'%s'"), + values[i]); + } + } else if (STREQ(keywords[i], "heads")) { + if (virStrToLong_ui(values[i], NULL, 10, + &(def->geometry.heads)) < 0) { + virDomainDiskDefFree(def); + def = NULL; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse heads value'%s'"), + values[i]); + } + } else if (STREQ(keywords[i], "secs")) { + if (virStrToLong_ui(values[i], NULL, 10, + &(def->geometry.sectors)) < 0) { + virDomainDiskDefFree(def); + def = NULL; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse sectors value'%s'"), + values[i]); + } + } else if (STREQ(keywords[i], "trans")) { + def->geometry.trans = + virDomainDiskGeometryTransTypeFromString(values[i]); + if ((trans < VIR_DOMAIN_DISK_TRANS_DEFAULT) || + (trans >= VIR_DOMAIN_DISK_TRANS_LAST)) { + virDomainDiskDefFree(def); + def = NULL; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse translation value'%s'"), + values[i]); + } } }
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 08/20/2012 02:06 PM, Daniel P. Berrange wrote:
On Mon, Aug 20, 2012 at 01:58:25PM +0200, Viktor Mihajlovski wrote:
From: J.B. Joret <jb@linux.vnet.ibm.com>
Qemu command line generation for geometry override and testcases.
V2 Changes: squashed qemu code and testcases.
V3 Changes: use virReportError.
V4 Changes: rebase
V5 Changes: Fixed test invocation for geometry.
Signed-off-by: J.B. Joret <jb@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 59 ++++++++++++++++++++ .../qemuxml2argv-disk-geometry.args | 4 + .../qemuxml2argv-disk-geometry.xml | 26 +++++++++ tests/qemuxml2argvtest.c | 2 + 4 files changed, 91 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9383530..cc44015 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2073,6 +2073,8 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, { virBuffer opt = VIR_BUFFER_INITIALIZER; const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); + const char *trans = + virDomainDiskGeometryTransTypeToString(disk->geometry.trans); int idx = virDiskNameToIndex(disk->dst); int busid = -1, unitid = -1;
@@ -2275,6 +2277,24 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, disk->type != VIR_DOMAIN_DISK_TYPE_DIR && qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT)) virBufferAsprintf(&opt, ",format=%s", disk->driverType); + + /* generate geometry command string*/ + if (disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK || + disk->type == VIR_DOMAIN_DISK_TYPE_FILE) {
The cylinders/heads/sectors data is part of the guest side device model. The disk->type == block|file|network is part of the host side device model. IIUC, there should be no dependency between host and guest side data for this property, so IMHO this if() should be removed.
good catch. As the same suppression happens in domain_conf.c, I will resend the series. -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (2)
-
Daniel P. Berrange -
Viktor Mihajlovski