On Fri, Jul 13, 2012 at 10:45:28AM +0200, Viktor Mihajlovski wrote:
> From: J.B. Joret <jb@xxxxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Viktor Mihajlovski <mihajlov@xxxxxxxxxxxxxxxxxx>
> ---
> 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list