[Libvirt-cim] [PATCH] Add disk device='lun' support

[John Ferlan]

On 05/16/2014 03:40 AM, Xu Wang wrote:
> Besides 'disk', 'cdrom', 'floppy' and 'filesystem', there is one
> more value 'lun' should be supported by value of device field in
> the disk device. So this patch adds it into libvirt-cim. Now device
> like <disk type='block' device='lun'> could be operated by class.
> 
> Signed-off-by: Xu Wang <gesaint at linux.vnet.ibm.com>
> ---
>  schema/ResourceAllocationSettingData.mof  |    8 ++++----
>  src/Virt_RASD.c                           |    3 +++
>  src/Virt_RASD.h                           |    1 +
>  src/Virt_SettingsDefineCapabilities.c     |    2 ++
>  src/Virt_VirtualSystemManagementService.c |    2 ++
>  5 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/schema/ResourceAllocationSettingData.mof b/schema/ResourceAllocationSettingData.mof
> index 9c387f0..f78d423 100644
> --- a/schema/ResourceAllocationSettingData.mof
> +++ b/schema/ResourceAllocationSettingData.mof
> @@ -10,8 +10,8 @@ class Xen_DiskResourceAllocationSettingData : Xen_ResourceAllocationSettingData
>        string VirtualDevice;
>  
>        [Description ("Device emulation type"),
> -        ValueMap {"0", "1", "2"},
> -        Values {"Disk", "CDROM", "floppy"}]
> +        ValueMap {"0", "1", "2", "4"},
> +        Values {"Disk", "CDROM", "floppy", "lun"}]
>        uint16 EmulatedType;
>  
>        [Description ("Bus type of the device")]
> @@ -43,8 +43,8 @@ class KVM_DiskResourceAllocationSettingData : KVM_ResourceAllocationSettingData
>        string VirtualDevice;
>  
>        [Description ("Device emulation type"),
> -        ValueMap {"0", "1", "2", "3"},
> -        Values {"Disk", "CDROM", "floppy", "filesystem"}]
> +        ValueMap {"0", "1", "2", "3", "4"},
> +        Values {"Disk", "CDROM", "floppy", "filesystem", "lun"}]
>        uint16 EmulatedType;
>  
>        [Description ("Bus type of the device")]
> diff --git a/src/Virt_RASD.c b/src/Virt_RASD.c
> index 3c62c2d..761d053 100644
> --- a/src/Virt_RASD.c
> +++ b/src/Virt_RASD.c
> @@ -444,6 +444,9 @@ static CMPIStatus set_disk_rasd_params(const CMPIBroker *broker,
>          else if ((dev->dev.disk.device != NULL) &&
>                  STREQ(dev->dev.disk.device, "floppy"))
>                  type = VIRT_DISK_TYPE_FLOPPY;
> +        else if ((dev->dev.disk.device != NULL) &&
> +                STREQ(dev->dev.disk.device, "lun"))
> +                type = VIRT_DISK_TYPE_LUN;
>          else
>                  type = VIRT_DISK_TYPE_DISK;
>  
> diff --git a/src/Virt_RASD.h b/src/Virt_RASD.h
> index 400143f..49d7195 100644
> --- a/src/Virt_RASD.h
> +++ b/src/Virt_RASD.h
> @@ -27,6 +27,7 @@
>  #define VIRT_DISK_TYPE_CDROM 1
>  #define VIRT_DISK_TYPE_FLOPPY 2
>  #define VIRT_DISK_TYPE_FS 3
> +#define VIRT_DISK_TYPE_LUN 4
>  
>  char *rasd_to_xml(CMPIInstance *rasd);
>  
> diff --git a/src/Virt_SettingsDefineCapabilities.c b/src/Virt_SettingsDefineCapabilities.c
> index 756e46b..714153e 100644
> --- a/src/Virt_SettingsDefineCapabilities.c
> +++ b/src/Virt_SettingsDefineCapabilities.c
> @@ -1078,6 +1078,8 @@ static CMPIStatus set_disk_props(int type,
>                          dev = "hdc";
>                  } else if (emu_type == VIRT_DISK_TYPE_FLOPPY) {
>                          dev = "fda";
> +                } else if (emu_type == VIRT_DISK_TYPE_LUN) {
> +                        dev = "sda";

Are there any other properties that should be set here?  I see that
TYPE_DISK sets "VirtualQuantity" which is something I would think a LUN
would have.
>                  }
>  
>                  CMSetProperty(inst, "VirtualDevice",
> diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c
> index f673c38..28a9a5f 100644
> --- a/src/Virt_VirtualSystemManagementService.c
> +++ b/src/Virt_VirtualSystemManagementService.c
> @@ -1149,6 +1149,8 @@ static const char *disk_rasd_to_vdev(CMPIInstance *inst,

@ line 1116 there's a check for type == VIRT_DISK_TYPE_DISK || TYPE_FS
with some sort of sub test for disk_type - is that something that
perhaps needs to be checked for LUN as well?

>                  dev->dev.disk.device = strdup("floppy");
>          else if (type == VIRT_DISK_TYPE_FS) 
>                  dev->dev.disk.device = strdup("filesystem");
> +        else if (type == VIRT_DISK_TYPE_LUN)
> +                dev->dev.disk.device = strdup("lun");
>          else
>                  return "Invalid value for EmulatedType";
>  
> 

My cimtest run went fine, which is good!

So just answer the two inquiries and I can push this.  If necessary I
can squash whatever you want in...

John