[libvirt] [PATCH V2 1/2] libxl: set default disk format in device post-parse

Jim Fehlig posted 2 patches 9 years ago
[libvirt] [PATCH V2 1/2] libxl: set default disk format in device post-parse
Posted by Jim Fehlig 9 years ago
When starting a domian, a libxl_domain_config object is created from
virDomainDef. Any virDomainDiskDef devices with a format of
VIR_STORAGE_FILE_NONE are mapped to LIBXL_DISK_FORMAT_RAW in the
corresponding libxl_disk_device, but the virDomainDiskDef format is
never updated to reflect the change.

A better place to set a default format for disk devices is the
device post-parse callback, ensuring the virDomainDiskDef object
reflects the default format.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/libxl/libxl_conf.c   | 10 ++--------
 src/libxl/libxl_domain.c |  7 ++++++-
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index b5186f2..da63105 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -765,8 +765,6 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
                 x_disk->format = LIBXL_DISK_FORMAT_VHD;
                 x_disk->backend = LIBXL_DISK_BACKEND_TAP;
                 break;
-            case VIR_STORAGE_FILE_NONE:
-                /* No subtype specified, default to raw/tap */
             case VIR_STORAGE_FILE_RAW:
                 x_disk->format = LIBXL_DISK_FORMAT_RAW;
                 x_disk->backend = LIBXL_DISK_BACKEND_TAP;
@@ -802,8 +800,6 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
             case VIR_STORAGE_FILE_VHD:
                 x_disk->format = LIBXL_DISK_FORMAT_VHD;
                 break;
-            case VIR_STORAGE_FILE_NONE:
-                /* No subtype specified, default to raw */
             case VIR_STORAGE_FILE_RAW:
                 x_disk->format = LIBXL_DISK_FORMAT_RAW;
                 break;
@@ -816,8 +812,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
                 return -1;
             }
         } else if (STREQ(driver, "file")) {
-            if (format != VIR_STORAGE_FILE_NONE &&
-                format != VIR_STORAGE_FILE_RAW) {
+            if (format != VIR_STORAGE_FILE_RAW) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("libxenlight does not support disk format %s "
                                  "with disk driver %s"),
@@ -828,8 +823,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
             x_disk->format = LIBXL_DISK_FORMAT_RAW;
             x_disk->backend = LIBXL_DISK_BACKEND_QDISK;
         } else if (STREQ(driver, "phy")) {
-            if (format != VIR_STORAGE_FILE_NONE &&
-                format != VIR_STORAGE_FILE_RAW) {
+            if (format != VIR_STORAGE_FILE_RAW) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("libxenlight does not support disk format %s "
                                  "with disk driver %s"),
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index ed73cd2..c0ace33 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -362,16 +362,21 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
         }
     }
 
-    /* for network-based disks, set 'qemu' as the default driver */
     if (dev->type == VIR_DOMAIN_DEVICE_DISK) {
         virDomainDiskDefPtr disk = dev->data.disk;
         int actual_type = virStorageSourceGetActualType(disk->src);
+        int format = virDomainDiskGetFormat(disk);
 
+        /* for network-based disks, set 'qemu' as the default driver */
         if (actual_type == VIR_STORAGE_TYPE_NETWORK) {
             if (!virDomainDiskGetDriver(disk) &&
                 virDomainDiskSetDriver(disk, "qemu") < 0)
                 return -1;
         }
+
+        /* xl.cfg default format is raw. See xl-disk-configuration(5) */
+        if (format == VIR_STORAGE_FILE_NONE)
+            virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
     }
 
     return 0;
-- 
2.9.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2 1/2] libxl: set default disk format in device post-parse
Posted by Michal Privoznik 9 years ago
On 02/07/2017 08:46 PM, Jim Fehlig wrote:
> When starting a domian, a libxl_domain_config object is created from
> virDomainDef. Any virDomainDiskDef devices with a format of
> VIR_STORAGE_FILE_NONE are mapped to LIBXL_DISK_FORMAT_RAW in the
> corresponding libxl_disk_device, but the virDomainDiskDef format is
> never updated to reflect the change.
> 
> A better place to set a default format for disk devices is the
> device post-parse callback, ensuring the virDomainDiskDef object
> reflects the default format.
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
>  src/libxl/libxl_conf.c   | 10 ++--------
>  src/libxl/libxl_domain.c |  7 ++++++-
>  2 files changed, 8 insertions(+), 9 deletions(-)

ACK.

BTW looks like you forgot about one other location:

src/libxl/libxl_driver.c=5452=libxlDomainBlockStatsGatherSingle(virDomainObjPtr vm,
--
src/libxl/libxl_driver.c-5469-
src/libxl/libxl_driver.c-5470-    if (STREQ(disk_drv, "phy")) {
src/libxl/libxl_driver.c-5471-        if (disk_fmt != VIR_STORAGE_FILE_RAW &&
src/libxl/libxl_driver.c:5472:            disk_fmt != VIR_STORAGE_FILE_NONE) {
src/libxl/libxl_driver.c-5473-            virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
src/libxl/libxl_driver.c-5474-                           _("unsupported format %s"),
src/libxl/libxl_driver.c-5475-                           virStorageFileFormatTypeToString(disk_fmt));


Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2 1/2] libxl: set default disk format in device post-parse
Posted by Jim Fehlig 9 years ago
Michal Privoznik wrote:
> On 02/07/2017 08:46 PM, Jim Fehlig wrote:
>> When starting a domian, a libxl_domain_config object is created from
>> virDomainDef. Any virDomainDiskDef devices with a format of
>> VIR_STORAGE_FILE_NONE are mapped to LIBXL_DISK_FORMAT_RAW in the
>> corresponding libxl_disk_device, but the virDomainDiskDef format is
>> never updated to reflect the change.
>>
>> A better place to set a default format for disk devices is the
>> device post-parse callback, ensuring the virDomainDiskDef object
>> reflects the default format.
>>
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>> ---
>>  src/libxl/libxl_conf.c   | 10 ++--------
>>  src/libxl/libxl_domain.c |  7 ++++++-
>>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> ACK.
> 
> BTW looks like you forgot about one other location:
> 
> src/libxl/libxl_driver.c=5452=libxlDomainBlockStatsGatherSingle(virDomainObjPtr vm,
> --
> src/libxl/libxl_driver.c-5469-
> src/libxl/libxl_driver.c-5470-    if (STREQ(disk_drv, "phy")) {
> src/libxl/libxl_driver.c-5471-        if (disk_fmt != VIR_STORAGE_FILE_RAW &&
> src/libxl/libxl_driver.c:5472:            disk_fmt != VIR_STORAGE_FILE_NONE) {
> src/libxl/libxl_driver.c-5473-            virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> src/libxl/libxl_driver.c-5474-                           _("unsupported format %s"),
> src/libxl/libxl_driver.c-5475-                           virStorageFileFormatTypeToString(disk_fmt));

Ah, thanks. I've added that one to the patch and pushed the series.

Regards,
Jim

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list