[libvirt] [PATCH 0/3] Allow to override disk geometry.
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. J.B. Joret (3): qemu: Support for Disk Geometry Override qemu: Documentation for Disk Geometry Override qemu: Testcase for Disk Geometry Override docs/formatdomain.html.in | 25 +++++++ docs/schemas/domaincommon.rng | 28 ++++++++ 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, 234 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> Qemu allows to override the disk geometry in the drive specification with cyls=,heads=,secs=[,trans=]. This patch extends the domain config and the qemu driver to allow the specification of disk geometry with libvirt. Signed-off-by: J.B. Joret <jb@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 17 +++++++++++ src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 59 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 149 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4f8c57a..4b208fc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -170,6 +170,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", @@ -3347,6 +3353,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; @@ -3375,6 +3382,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"); @@ -3484,6 +3496,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) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid geometry settings (cyls)")); + goto error; + } + if (virXPathUInt("string(./geometry/@heads)", + ctxt, &def->geometry.heads) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid geometry settings (heads)")); + goto error; + } + if (virXPathUInt("string(./geometry/@secs)", + ctxt, &def->geometry.sectors) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("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 { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid translation value '%s'"), + trans); + goto error; + } + } } else if (!driverName && xmlStrEqual(cur->name, BAD_CAST "driver")) { driverName = virXMLPropString(cur, "name"); @@ -3962,6 +4008,7 @@ cleanup: VIR_FREE(target); VIR_FREE(source); VIR_FREE(tray); + VIR_FREE(trans); while (nhosts > 0) { virDomainDiskHostDefFree(&hosts[nhosts - 1]); nhosts--; @@ -10973,6 +11020,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, @@ -11094,6 +11163,7 @@ virDomainDiskDefFormat(virBufferPtr buf, } else { virBufferAddLit(buf, "/>\n"); } + virDomainDiskGeometryDefFormat(buf, def); break; case VIR_DOMAIN_DISK_TYPE_BLOCK: virBufferEscapeString(buf, " <source dev='%s'", @@ -11107,6 +11177,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 5e5374a..018a1f1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -466,6 +466,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 { @@ -575,6 +584,13 @@ struct _virDomainDiskDef { char *mirrorFormat; bool mirroring; + struct { + unsigned int cylinders; + unsigned int heads; + unsigned int sectors; + int trans; + } geometry; + virDomainBlockIoTuneInfo blkdeviotune; char *serial; @@ -2165,6 +2181,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 b173590..6b2064e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -304,6 +304,8 @@ virDomainDiskDefAssignAddress; virDomainDiskDefForeachPath; virDomainDiskDefFree; virDomainDiskDeviceTypeToString; +virDomainDiskGeometryTransTypeToString; +virDomainDiskGeometryTransTypeFromString; virDomainDiskErrorPolicyTypeFromString; virDomainDiskErrorPolicyTypeToString; virDomainDiskFindControllerModel; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ae48678..23d1aba 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2016,6 +2016,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; @@ -2218,6 +2220,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) @@ -6599,6 +6619,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, @@ -6803,6 +6824,44 @@ qemuParseCommandLineDisk(virCapsPtr caps, qemuReportError(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; + qemuReportError(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; + qemuReportError(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; + qemuReportError(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; + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse translation value'%s'"), + values[i]); + } } } -- 1.7.0.4
On Fri, Jul 13, 2012 at 10:45:28AM +0200, Viktor Mihajlovski wrote:
From: J.B. Joret <jb@linux.vnet.ibm.com>
Qemu allows to override the disk geometry in the drive specification with cyls=,heads=,secs=[,trans=]. This patch extends the domain config and the qemu driver to allow the specification of disk geometry with libvirt.
Signed-off-by: J.B. Joret <jb@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 17 +++++++++++ src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 59 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 149 insertions(+), 0 deletions(-)
Since you're adding to the public XML schema, please also update the files docs/schemas/domaincommon.rng and docs/formatdomain.html.in with appropriate info. It would also be desirable to add some test cases to the qemuxml2argvtest.c test to validate this.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4f8c57a..4b208fc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -170,6 +170,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", @@ -3347,6 +3353,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; @@ -3375,6 +3382,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"); @@ -3484,6 +3496,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) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid geometry settings (cyls)")); + goto error; + } + if (virXPathUInt("string(./geometry/@heads)", + ctxt, &def->geometry.heads) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid geometry settings (heads)")); + goto error; + } + if (virXPathUInt("string(./geometry/@secs)", + ctxt, &def->geometry.sectors) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("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 { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid translation value '%s'"), + trans); + goto error;
Since you went to trouble of defining VIR_ENUM, you should be using virDomainDiskGeometryTransTypeFromString() here instead of all these STREQs.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ae48678..23d1aba 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2016,6 +2016,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;
@@ -2218,6 +2220,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 geometry information is used to set guest device properties. The disk->type setting is about the host backend image type. So I don't see why there's a dependancy between them. IMHO you should just remove this if, and unconditionally set the geometry regardless of what the backend type is. Regards, 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> Updates to domain schema definition and the domain XML documentation for the disk geometry specification. 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 | 28 ++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 0 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b6e0d5d..7f42753 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1222,6 +1222,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> @@ -1569,6 +1575,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 b7562ad..b6970d9 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -945,6 +945,9 @@ </optional> </element> </optional> + <optional> + <ref name="geometry"/> + </optional> <ref name="diskspec"/> </interleave> </group> @@ -963,6 +966,9 @@ </optional> </element> </optional> + <optional> + <ref name="geometry"/> + </optional> <ref name="diskspec"/> </interleave> </group> @@ -1053,6 +1059,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. --> -- 1.7.0.4
On Fri, Jul 13, 2012 at 10:45:29AM +0200, Viktor Mihajlovski wrote:
From: J.B. Joret <jb@linux.vnet.ibm.com>
Updates to domain schema definition and the domain XML documentation for the disk geometry specification.
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 | 28 ++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 0 deletions(-)
Ahhh, this patch should actually be part of the first patch - always add the schema/docs in the same patch the implements the parsing 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> Adding qemuxml2arg test for disk geometry in drive specification. Signed-off-by: J.B. Joret <jb@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- .../qemuxml2argv-disk-geometry.args | 4 +++ .../qemuxml2argv-disk-geometry.xml | 26 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 + 3 files changed, 32 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.xml 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 a4fa8fe..1a49c7c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -776,6 +776,8 @@ mymain(void) QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD); + DO_TEST("disk-geometry", false, QEMU_CAPS_DRIVE); + VIR_FREE(driver.stateDir); virCapabilitiesFree(driver.caps); VIR_FREE(map); -- 1.7.0.4
On Fri, Jul 13, 2012 at 10:45:30AM +0200, Viktor Mihajlovski wrote:
From: J.B. Joret <jb@linux.vnet.ibm.com>
Adding qemuxml2arg test for disk geometry in drive specification.
Signed-off-by: J.B. Joret <jb@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- .../qemuxml2argv-disk-geometry.args | 4 +++ .../qemuxml2argv-disk-geometry.xml | 26 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 + 3 files changed, 32 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.xml
And this answers one of my other points about the first patch. It would be better if you move the qemu_command.c change out of the first patch, and into this patch. In others you wnat to end up with a total of 2 patches 1. The XML parsing, formatting, schema additions and docs 2. The QEMU command impl and the QEMU tests 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 :|
Thanks for reviewing the patches. V2 of the series refactors the changes into the generic domain configuration/XML support for disk geometry and a qemu specific part with the testcases. Kind Regards, Viktor J.B. Joret (2): Support for Disk Geometry Override qemu: Disk Geometry Override Support docs/formatdomain.html.in | 25 +++++++ docs/schemas/domaincommon.rng | 28 ++++++++ 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, 234 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. 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 | 28 ++++++++++++++++ src/conf/domain_conf.c | 71 +++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 17 ++++++++++ src/libvirt_private.syms | 2 + 5 files changed, 143 insertions(+), 0 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b6e0d5d..7f42753 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1222,6 +1222,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> @@ -1569,6 +1575,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 b7562ad..b6970d9 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -945,6 +945,9 @@ </optional> </element> </optional> + <optional> + <ref name="geometry"/> + </optional> <ref name="diskspec"/> </interleave> </group> @@ -963,6 +966,9 @@ </optional> </element> </optional> + <optional> + <ref name="geometry"/> + </optional> <ref name="diskspec"/> </interleave> </group> @@ -1053,6 +1059,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 4f8c57a..4b208fc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -170,6 +170,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", @@ -3347,6 +3353,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; @@ -3375,6 +3382,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"); @@ -3484,6 +3496,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) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid geometry settings (cyls)")); + goto error; + } + if (virXPathUInt("string(./geometry/@heads)", + ctxt, &def->geometry.heads) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid geometry settings (heads)")); + goto error; + } + if (virXPathUInt("string(./geometry/@secs)", + ctxt, &def->geometry.sectors) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("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 { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid translation value '%s'"), + trans); + goto error; + } + } } else if (!driverName && xmlStrEqual(cur->name, BAD_CAST "driver")) { driverName = virXMLPropString(cur, "name"); @@ -3962,6 +4008,7 @@ cleanup: VIR_FREE(target); VIR_FREE(source); VIR_FREE(tray); + VIR_FREE(trans); while (nhosts > 0) { virDomainDiskHostDefFree(&hosts[nhosts - 1]); nhosts--; @@ -10973,6 +11020,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, @@ -11094,6 +11163,7 @@ virDomainDiskDefFormat(virBufferPtr buf, } else { virBufferAddLit(buf, "/>\n"); } + virDomainDiskGeometryDefFormat(buf, def); break; case VIR_DOMAIN_DISK_TYPE_BLOCK: virBufferEscapeString(buf, " <source dev='%s'", @@ -11107,6 +11177,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 5e5374a..018a1f1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -466,6 +466,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 { @@ -575,6 +584,13 @@ struct _virDomainDiskDef { char *mirrorFormat; bool mirroring; + struct { + unsigned int cylinders; + unsigned int heads; + unsigned int sectors; + int trans; + } geometry; + virDomainBlockIoTuneInfo blkdeviotune; char *serial; @@ -2165,6 +2181,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 b173590..6b2064e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -304,6 +304,8 @@ virDomainDiskDefAssignAddress; virDomainDiskDefForeachPath; virDomainDiskDefFree; virDomainDiskDeviceTypeToString; +virDomainDiskGeometryTransTypeToString; +virDomainDiskGeometryTransTypeFromString; virDomainDiskErrorPolicyTypeFromString; virDomainDiskErrorPolicyTypeToString; virDomainDiskFindControllerModel; -- 1.7.0.4
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. Signed-off-by: J.B. Joret <jb@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 ae48678..23d1aba 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2016,6 +2016,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; @@ -2218,6 +2220,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) @@ -6599,6 +6619,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, @@ -6803,6 +6824,44 @@ qemuParseCommandLineDisk(virCapsPtr caps, qemuReportError(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; + qemuReportError(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; + qemuReportError(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; + qemuReportError(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; + qemuReportError(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 a4fa8fe..1a49c7c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -776,6 +776,8 @@ mymain(void) QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD); + DO_TEST("disk-geometry", false, QEMU_CAPS_DRIVE); + VIR_FREE(driver.stateDir); virCapabilitiesFree(driver.caps); VIR_FREE(map); -- 1.7.0.4
Two more changes: 1. Use virReportError instead of custom xxxReportError. 2. Moved the geometry element to diskspec, so it applies to all kind of disks. Please disregard the preceding patches. 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 b6e0d5d..7f42753 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1222,6 +1222,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> @@ -1569,6 +1575,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 b7562ad..dd6496e 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -851,6 +851,9 @@ <optional> <ref name="address"/> </optional> + <optional> + <ref name="geometry"/> + </optional> </interleave> </define> <define name="snapshot"> @@ -1053,6 +1056,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 1b5dad9..0c4bdc9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -170,6 +170,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", @@ -3343,6 +3349,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; @@ -3371,6 +3378,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"); @@ -3480,6 +3492,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"); @@ -3958,6 +4004,7 @@ cleanup: VIR_FREE(target); VIR_FREE(source); VIR_FREE(tray); + VIR_FREE(trans); while (nhosts > 0) { virDomainDiskHostDefFree(&hosts[nhosts - 1]); nhosts--; @@ -10976,6 +11023,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, @@ -11097,6 +11166,7 @@ virDomainDiskDefFormat(virBufferPtr buf, } else { virBufferAddLit(buf, "/>\n"); } + virDomainDiskGeometryDefFormat(buf, def); break; case VIR_DOMAIN_DISK_TYPE_BLOCK: virBufferEscapeString(buf, " <source dev='%s'", @@ -11110,6 +11180,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 6c777e7..ae5ae3c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -466,6 +466,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 { @@ -575,6 +584,13 @@ struct _virDomainDiskDef { char *mirrorFormat; bool mirroring; + struct { + unsigned int cylinders; + unsigned int heads; + unsigned int sectors; + int trans; + } geometry; + virDomainBlockIoTuneInfo blkdeviotune; char *serial; @@ -2165,6 +2181,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 734c881..d6dd166 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -307,6 +307,8 @@ virDomainDiskDefAssignAddress; virDomainDiskDefForeachPath; virDomainDiskDefFree; virDomainDiskDeviceTypeToString; +virDomainDiskGeometryTransTypeToString; +virDomainDiskGeometryTransTypeFromString; virDomainDiskErrorPolicyTypeFromString; virDomainDiskErrorPolicyTypeToString; virDomainDiskFindControllerModel; -- 1.7.0.4
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. Signed-off-by: J.B. Joret <jb@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 d34af96..30dfcbd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2016,6 +2016,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; @@ -2218,6 +2220,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) @@ -6599,6 +6619,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, @@ -6803,6 +6824,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 a4fa8fe..1a49c7c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -776,6 +776,8 @@ mymain(void) QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD); + DO_TEST("disk-geometry", false, QEMU_CAPS_DRIVE); + VIR_FREE(driver.stateDir); virCapabilitiesFree(driver.caps); VIR_FREE(map); -- 1.7.0.4
Obsoleted by new patch set. -- 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