[libvirt] [PATCHv5 2/2] qemu: Disk Geometry Override Support

[Daniel P. Berrange]

On Mon, Aug 20, 2012 at 01:58:25PM +0200, Viktor Mihajlovski wrote:
> From: J.B. Joret <jb at 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 at linux.vnet.ibm.com>
> Signed-off-by: Viktor Mihajlovski <mihajlov at linux.vnet.ibm.com>
> 
> Signed-off-by: Viktor Mihajlovski <mihajlov at 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 :|