[Libvirt-cim] [PATCH] Add shareable property to disk

[WangXu]

----------------------------------------
> Date: Thu, 25 Jul 2013 15:33:14 +0800
> From: veillard at redhat.com
> To: libvirt-cim at redhat.com
> Subject: Re: [Libvirt-cim] [PATCH] Add shareable property to disk
>
> On Wed, Jul 24, 2013 at 10:49:19AM +0800, cngesaint at gmail.com wrote:
>> From: Xu Wang <cngesaint at gmail.com>
>>
>> This patch adds shareable property to disks.
>>
>> Signed-off-by: Xu Wang <cngesaint at gmail.com>
>> ---
>> schema/ResourceAllocationSettingData.mof | 6 ++++++
>> src/Virt_RASD.c | 6 ++++++
>> src/Virt_SettingsDefineCapabilities.c | 14 ++++++++++++++
>> src/Virt_VirtualSystemManagementService.c | 6 ++++++
>> 4 files changed, 32 insertions(+), 0 deletions(-)
>>
>> diff --git a/schema/ResourceAllocationSettingData.mof b/schema/ResourceAllocationSettingData.mof
>> index 108dff7..871ab04 100644
>> --- a/schema/ResourceAllocationSettingData.mof
>> +++ b/schema/ResourceAllocationSettingData.mof
>> @@ -28,6 +28,9 @@ class Xen_DiskResourceAllocationSettingData : Xen_ResourceAllocationSettingData
>>
>> [Description ("cache setting for device")]
>> string DriverCache;
>> +
>> + [Description ("if device is shareable")]
>> + boolean shareable;
>> };
>>
>> [Description ("KVM virtual disk configuration"),
>> @@ -61,6 +64,9 @@ class KVM_DiskResourceAllocationSettingData : KVM_ResourceAllocationSettingData
>>
>> [Description ("filesystem access mode")]
>> string AccessMode;
>> +
>> + [Description ("if device is shareable")]
>> + boolean shareable;
>> };
>>
>> [Description ("LXC virtual disk configuration"),
>> diff --git a/src/Virt_RASD.c b/src/Virt_RASD.c
>> index baf4331..150ccd3 100644
>> --- a/src/Virt_RASD.c
>> +++ b/src/Virt_RASD.c
>> @@ -421,6 +421,12 @@ static CMPIStatus set_disk_rasd_params(const CMPIBroker *broker,
>> (CMPIValue *)dev->dev.disk.access_mode,
>> CMPI_chars);
>>
>> + if(dev->dev.disk.shareable)
>> + CMSetProperty(inst,
>> + "shareable",
>> + (CMPIValue *)&(dev->dev.disk.shareable),
>> + CMPI_boolean);
>> +
>> virStoragePoolFree(pool);
>> virStorageVolFree(vol);
>> virConnectClose(conn);
>> diff --git a/src/Virt_SettingsDefineCapabilities.c b/src/Virt_SettingsDefineCapabilities.c
>> index 2c35d84..78c128c 100644
>> --- a/src/Virt_SettingsDefineCapabilities.c
>> +++ b/src/Virt_SettingsDefineCapabilities.c
>> @@ -1039,6 +1039,7 @@ static CMPIStatus set_disk_props(int type,
>> uint64_t disk_size,
>> uint16_t emu_type,
>> bool readonly,
>> + bool shareable,
>> const char *cache,
>> struct inst_list *list)
>> {
>> @@ -1087,6 +1088,10 @@ static CMPIStatus set_disk_props(int type,
>> CMSetProperty(inst, "readonly",
>> (CMPIValue *)&readonly, CMPI_boolean);
>>
>> + if(shareable)
>> + CMSetProperty(inst, "shareable",
>> + (CMPIValue *)&shareable, CMPI_boolean);
>> +
>> if(cache != NULL)
>> CMSetProperty(inst, "cache",
>> (CMPIValue *)cache, CMPI_chars);
>> @@ -1111,6 +1116,7 @@ static CMPIStatus cdrom_or_floppy_template(const CMPIObjectPath *ref,
>> const char *dev_str = NULL;
>> char *id_str = NULL;
>> bool readonly = true;
>> + bool shareable = false;
>> const char *cache = "none";
>>
>> if (emu_type == VIRT_DISK_TYPE_CDROM)
>> @@ -1158,6 +1164,7 @@ static CMPIStatus cdrom_or_floppy_template(const CMPIObjectPath *ref,
>> vol_size,
>> emu_type,
>> readonly,
>> + shareable,
>> cache,
>> list);
>> }
>> @@ -1169,6 +1176,7 @@ static CMPIStatus cdrom_or_floppy_template(const CMPIObjectPath *ref,
>> vol_size,
>> emu_type,
>> readonly,
>> + shareable,
>> cache,
>> list);
>>
>> @@ -1244,6 +1252,7 @@ static CMPIStatus default_disk_template(const CMPIObjectPath *ref,
>> int type = 0;
>> bool ret;
>> bool readonly = true;
>> + bool shareable = false;
>> const char *cache = "none";
>>
>> CMPIStatus s = {CMPI_RC_OK, NULL};
>> @@ -1296,6 +1305,7 @@ static CMPIStatus default_disk_template(const CMPIObjectPath *ref,
>> disk_size,
>> emu_type,
>> readonly,
>> + shareable,
>> cache,
>> list);
>> if (s.rc != CMPI_RC_OK)
>> @@ -1317,6 +1327,7 @@ static CMPIStatus default_disk_template(const CMPIObjectPath *ref,
>> disk_size,
>> emu_type,
>> readonly,
>> + shareable,
>> cache,
>> list);
>> }
>> @@ -1444,6 +1455,7 @@ static CMPIStatus avail_volume_template(const CMPIObjectPath *ref,
>> int ret;
>> uint16_t emu_type = 0;
>> bool readonly = false;
>> + bool shareable = false;
>> const char *cache = "none";
>>
>> ret = virStorageVolGetInfo(volume_ptr, &vol_info);
>> @@ -1502,6 +1514,7 @@ static CMPIStatus avail_volume_template(const CMPIObjectPath *ref,
>> vol_size,
>> emu_type,
>> readonly,
>> + shareable,
>> cache,
>> list);
>> }
>> @@ -1513,6 +1526,7 @@ static CMPIStatus avail_volume_template(const CMPIObjectPath *ref,
>> vol_size,
>> emu_type,
>> readonly,
>> + shareable,
>> cache,
>> list);
>> } else {
>> diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c
>> index 5adfe20..8ced2d6 100644
>> --- a/src/Virt_VirtualSystemManagementService.c
>> +++ b/src/Virt_VirtualSystemManagementService.c
>> @@ -994,6 +994,7 @@ static const char *disk_rasd_to_vdev(CMPIInstance *inst,
>> uint16_t type;
>> bool read = false;
>> int rc;
>> + bool shareable = false;
>>
>> CU_DEBUG("Enter disk_rasd_to_vdev");
>> if (cu_get_str_prop(inst, "VirtualDevice", &val) != CMPI_RC_OK)
>> @@ -1093,6 +1094,11 @@ static const char *disk_rasd_to_vdev(CMPIInstance *inst,
>> else
>> dev->dev.disk.access_mode = strdup(val);
>>
>> + if (cu_get_bool_prop(inst, "shareable", &shareable) != CMPI_RC_OK)
>> + dev->dev.disk.shareable = false;
>> + else
>> + dev->dev.disk.shareable = shareable;
>> +
>> free(dev->id);
>> dev->id = strdup(dev->dev.disk.virtual_dev);
>>
>> --
>> 1.7.1
>
> This looks rather simple. My only worry is if this could change some
> of the public ABI, but exposing that property is important, and in the
> case of breaking ABI I would rather do it now than later,
>
> So I'm pushing this patch now, before the release, hopefully this
> is the right decision...
>
> thanks,
>
> Daniel
I made this patch because some others reported a bug and there is such
a request to need shareable added into public API. There was part of it
implemented before I made my patch. If shareable tag was added, it would
not effect the old users and satisfied the new request from users. So please
don't worry about that. If some others have better suggestions I'd like to
take them into consideration :-)

Xu Wang
>
> --
> Daniel Veillard | Open Source and Standards, Red Hat
> veillard at redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
> http://veillard.com/ | virtualization library http://libvirt.org/
>
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim