[libvirt] [PATCHv6 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. V6 Changes: The geometry info was incorrectly processed only for block and file based disks. Fixed. 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 | 56 +++++++++++++++ .../qemuxml2argv-disk-geometry.args | 4 + .../qemuxml2argv-disk-geometry.xml | 26 +++++++ tests/qemuxml2argvtest.c | 2 + 9 files changed, 228 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. V6 Changes: Generate geometry for all host device types. 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 35e9f82..fc042d5 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 851284a..d8ac1ad 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -166,6 +166,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", @@ -3252,6 +3258,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; @@ -3280,6 +3287,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"); @@ -3389,6 +3401,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"); @@ -3867,6 +3913,7 @@ cleanup: VIR_FREE(target); VIR_FREE(source); VIR_FREE(tray); + VIR_FREE(trans); while (nhosts > 0) { virDomainDiskHostDefFree(&hosts[nhosts - 1]); nhosts--; @@ -11006,6 +11053,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, @@ -11174,6 +11243,8 @@ virDomainDiskDefFormat(virBufferPtr buf, } } + virDomainDiskGeometryDefFormat(buf, def); + /* For now, mirroring is currently output-only: we only output it * for live domains, therefore we ignore it on input except for * the internal parse on libvirtd restart. */ diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index fd0e89e..e550747 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -452,6 +452,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 { @@ -561,6 +570,13 @@ struct _virDomainDiskDef { char *mirrorFormat; bool mirroring; + struct { + unsigned int cylinders; + unsigned int heads; + unsigned int sectors; + int trans; + } geometry; + virDomainBlockIoTuneInfo blkdeviotune; char *serial; @@ -2152,6 +2168,7 @@ VIR_ENUM_DECL(virDomainDevice) VIR_ENUM_DECL(virDomainDeviceAddress) 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 7539edc..666fa22 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -320,6 +320,8 @@ virDomainDiskDefAssignAddress; virDomainDiskDefForeachPath; virDomainDiskDefFree; virDomainDiskDeviceTypeToString; +virDomainDiskGeometryTransTypeToString; +virDomainDiskGeometryTransTypeFromString; virDomainDiskErrorPolicyTypeFromString; virDomainDiskErrorPolicyTypeToString; virDomainDiskFindControllerModel; -- 1.7.0.4
On Mon, Aug 20, 2012 at 03:58:50PM +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.
V6 Changes: Generate geometry for all host device types.
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 :|
On 08/21/2012 10:26 AM, Daniel P. Berrange wrote:
On Mon, Aug 20, 2012 at 03:58:50PM +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.
V6 Changes: Generate geometry for all host device types.
We tend to put the patch changelog...
Signed-off-by: J.B. Joret <jb@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> ---
after the --- line, as then it doesn't pollute the git history. It's great for review purposes in email, but once the thread is over, no one cares any longer how many revisions it took to get to the patch in git.
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(-)
+ <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
Trailing '.'
+ <dl> + <dt><code>cyls</code></dt> + <dd>The <code>cyls</code> element is the
s/element/attribute/
+ 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>
Missing a 'since 0.10.0' designation.
+ 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;
Why is this open-coded, instead of using...
+++ b/src/libvirt_private.syms @@ -320,6 +320,8 @@ virDomainDiskDefAssignAddress; virDomainDiskDefForeachPath; virDomainDiskDefFree; virDomainDiskDeviceTypeToString; +virDomainDiskGeometryTransTypeToString; +virDomainDiskGeometryTransTypeFromString; virDomainDiskErrorPolicyTypeFromString;
the new TypeFromString function? Also, this isn't sorted.
ACK
Pushed with the following tweaks, and with a listing for you in AUTHORS; let me know if I need to alter the spelling that you prefer for name or email. diff --git i/AUTHORS w/AUTHORS index 5dec3a2..289c984 100644 --- i/AUTHORS +++ w/AUTHORS @@ -258,6 +258,8 @@ Patches have also been contributed by: ... diff --git i/docs/formatdomain.html.in w/docs/formatdomain.html.in index 5bb9a76..d87ca6b 100644 --- i/docs/formatdomain.html.in +++ w/docs/formatdomain.html.in @@ -1580,19 +1580,19 @@ <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 + S390 DASD-disks or older DOS-disks. <span class="since">0.10.0</span> <dl> <dt><code>cyls</code></dt> - <dd>The <code>cyls</code> element is the + <dd>The <code>cyls</code> attribute is the number of cylinders. </dd> <dt><code>heads</code></dt> - <dd>The <code>heads</code> element is the + <dd>The <code>heads</code> attribute is the number of heads. </dd> <dt><code>secs</code></dt> - <dd>The <code>secs</code> element is the + <dd>The <code>secs</code> attribute is the number of sectors per track. </dd> <dt><code>trans</code></dt> - <dd>The optional <code>trans</code> element is the + <dd>The optional <code>trans</code> attribute is the BIOS-Translation-Modus (none, lba or auto)</dd> </dl> </dd> diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c index 754b1f2..c516685 100644 --- i/src/conf/domain_conf.c +++ w/src/conf/domain_conf.c @@ -3520,14 +3520,9 @@ virDomainDiskDefParseXML(virCapsPtr caps, 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 { + if (trans) { + def->geometry.trans = virDomainDiskGeometryTransTypeFromString(trans); + if (def->geometry.trans <= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid translation value '%s'"), trans); diff --git i/src/libvirt_private.syms w/src/libvirt_private.syms index 054f550..b20a754 100644 --- i/src/libvirt_private.syms +++ w/src/libvirt_private.syms @@ -323,11 +323,11 @@ virDomainDiskDefAssignAddress; virDomainDiskDefForeachPath; virDomainDiskDefFree; virDomainDiskDeviceTypeToString; -virDomainDiskGeometryTransTypeToString; -virDomainDiskGeometryTransTypeFromString; virDomainDiskErrorPolicyTypeFromString; virDomainDiskErrorPolicyTypeToString; virDomainDiskFindControllerModel; +virDomainDiskGeometryTransTypeFromString; +virDomainDiskGeometryTransTypeToString; virDomainDiskIndexByName; virDomainDiskInsert; virDomainDiskInsertPreAlloced; -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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. V6 Changes: Remove conditional geometry handling. 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 | 56 ++++++++++++++++++++ .../qemuxml2argv-disk-geometry.args | 4 ++ .../qemuxml2argv-disk-geometry.xml | 26 +++++++++ tests/qemuxml2argvtest.c | 2 + 4 files changed, 88 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 4ca3047..cdc8e55 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2076,6 +2076,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; @@ -2278,6 +2280,21 @@ 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->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) @@ -6723,6 +6740,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, @@ -6927,6 +6945,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 03:58:51PM +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.
V6 Changes: Remove conditional geometry handling.
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 | 56 ++++++++++++++++++++ .../qemuxml2argv-disk-geometry.args | 4 ++ .../qemuxml2argv-disk-geometry.xml | 26 +++++++++ tests/qemuxml2argvtest.c | 2 + 4 files changed, 88 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.xml
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 :|
On 08/21/2012 10:27 AM, Daniel P. Berrange wrote:
On Mon, Aug 20, 2012 at 03:58:51PM +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.
V6 Changes: Remove conditional geometry handling.
Again, I trimmed the commit message.
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 | 56 ++++++++++++++++++++ .../qemuxml2argv-disk-geometry.args | 4 ++ .../qemuxml2argv-disk-geometry.xml | 26 +++++++++ tests/qemuxml2argvtest.c | 2 + 4 files changed, 88 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.xml
+ /* generate geometry command string*/
Space before */
+-no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0,cyls=16383,heads=16,secs=63,trans=lba \
Long line; I added \-newline to split it.
ACK
Pushed with this squashed in: diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c index cdc8e55..ca62f0c 100644 --- i/src/qemu/qemu_command.c +++ w/src/qemu/qemu_command.c @@ -2281,7 +2281,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT)) virBufferAsprintf(&opt, ",format=%s", disk->driverType); - /* generate geometry command string*/ + /* generate geometry command string */ if (disk->geometry.cylinders > 0 && disk->geometry.heads > 0 && disk->geometry.sectors > 0) { diff --git i/tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args w/tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args index 7cd6650..c0de2ed 100644 --- i/tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args +++ w/tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args @@ -1,4 +1,5 @@ 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 \ +-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 -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/21/2012 11:36 PM, Eric Blake wrote:
Pushed with this squashed in:
diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c index cdc8e55..ca62f0c 100644 --- i/src/qemu/qemu_command.c +++ w/src/qemu/qemu_command.c @@ -2281,7 +2281,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT)) virBufferAsprintf(&opt, ",format=%s", disk->driverType);
- /* generate geometry command string*/ + /* generate geometry command string */ if (disk->geometry.cylinders > 0 && disk->geometry.heads > 0 && disk->geometry.sectors > 0) { diff --git i/tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args w/tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args index 7cd6650..c0de2ed 100644 --- i/tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args +++ w/tests/qemuxml2argvdata/qemuxml2argv-disk-geometry.args @@ -1,4 +1,5 @@ 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 \ +-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
Thanks Eric. -- 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 (3)
-
Daniel P. Berrange -
Eric Blake -
Viktor Mihajlovski