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 :|