[libvirt] [PATCH RFC 1/5] conf: Introduce readonly to hostdev and change helper function
Han Cheng
hanc.fnst at cn.fujitsu.com
Wed Mar 6 12:38:27 UTC 2013
Thanks for your comments~ Please correct me if I'm wrong.
As I know, <source> in <hostdev> is parsed by
virDomainHostdevSubsys(Pci/Usb)DefParseXML. Everything else in <hostdev>
is parsed by virDomainDeviceInfoParseXML.
I add readonly follow this.
And this XML is tested by hostdev-scsi-readonly(named by your advice).
Other problems will be fixed by next version.
On 03/06/2013 01:40 PM, Osier Yang wrote:
> On 2013年03月04日 14:01, Han Cheng wrote:
>> The only parameter in -drive affect scsi-generic is readonly. Introduce
>> <readonly/> to<hostdev>.
>> The helper function to look up disk controller model may be used by scsi
>> hostdev. But it should be changed to use info.
>> ---
>> docs/schemas/domaincommon.rng | 5 +++++
>> src/conf/domain_conf.c | 18 ++++++++++++++----
>> src/conf/domain_conf.h | 6 ++++--
>> src/libvirt_private.syms | 2 +-
>> src/qemu/qemu_command.c | 4 ++--
>> 5 files changed, 26 insertions(+), 9 deletions(-)
>>
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index e7231cc..fbb4001 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -2898,6 +2898,11 @@
>> <ref name="alias"/>
>> </optional>
>> <optional>
>> +<element name='readonly'>
>> +<empty/>
>> +</element>
>> +</optional>
>> +<optional>
>> <ref name="deviceBoot"/>
>> </optional>
>> <optional>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 995cf0c..5e385e4 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -78,6 +78,7 @@ typedef enum {
>> VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES = (1<<18),
>> VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM = (1<<19),
>> VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT = (1<<20),
>> + VIR_DOMAIN_XML_INTERNAL_ALLOW_READONLY = (1<<21),
>
> I think the "readonly" tag for hostdev can just always be exposed,
> as don't see any special reason to keep it internally.
>
>> } virDomainXMLInternalFlags;
>>
>> VIR_ENUM_IMPL(virDomainTaint, VIR_DOMAIN_TAINT_LAST,
>> @@ -2173,6 +2174,8 @@ virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info, unsigned int flags)
>> return true;
>> if (info->bootIndex)
>> return true;
>> + if (info->readonly)
>> + return true;
>
> And why it's of DeviceInfo struct?
>
>> return false;
>> }
>>
>> @@ -2395,6 +2398,8 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
>> virDomainDeviceInfoPtr info,
>> unsigned int flags)
>> {
>> + if ((flags& VIR_DOMAIN_XML_INTERNAL_ALLOW_READONLY)&& info->readonly)
>> + virBufferAsprintf(buf, "<readonly/>\n");
>> if ((flags& VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT)&& info->bootIndex)
>> virBufferAsprintf(buf, "<boot order='%d'/>\n", info->bootIndex);
>>
>> @@ -2803,6 +2808,10 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
>> (flags& VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM)&&
>> xmlStrEqual(cur->name, BAD_CAST "rom")) {
>> rom = cur;
>> + } else if (info->readonly == 0&&
>> + (flags& VIR_DOMAIN_XML_INTERNAL_ALLOW_READONLY)&&
>> + xmlStrEqual(cur->name, BAD_CAST "readonly")) {
>> + info->readonly = 1;
>> }
>> }
>> cur = cur->next;
>> @@ -3291,8 +3300,8 @@ error:
>> }
>>
>> int
>> -virDomainDiskFindControllerModel(virDomainDefPtr def,
>> - virDomainDiskDefPtr disk,
>> +virDomainInfoFindControllerModel(virDomainDefPtr def,
>
> Not a good name. How about changing into:
>
> virDomainDeviceFindControllerModel.
>
>> + virDomainDeviceInfoPtr info,
>> int controllerType)
>> {
>> int model = -1;
>> @@ -3300,7 +3309,7 @@ virDomainDiskFindControllerModel(virDomainDefPtr def,
>>
>> for (i = 0; i< def->ncontrollers; i++) {
>> if (def->controllers[i]->type == controllerType&&
>> - def->controllers[i]->idx == disk->info.addr.drive.controller)
>> + def->controllers[i]->idx == info->addr.drive.controller)
>> model = def->controllers[i]->model;
>> }
>>
>> @@ -7838,7 +7847,8 @@ virDomainHostdevDefParseXML(const xmlNodePtr node,
>> if (def->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
>> if (virDomainDeviceInfoParseXML(node, bootMap, def->info,
>> flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT
>> - | VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM)< 0)
>> + | VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM
>> + | VIR_DOMAIN_XML_INTERNAL_ALLOW_READONLY)< 0)
>> goto error;
>> }
>>
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index 5828ae2..39c5849 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -286,6 +286,8 @@ struct _virDomainDeviceInfo {
>> /* bootIndex is only used for disk, network interface, hostdev
>> * and redirdev devices */
>> int bootIndex;
>> + /* readonly is only used for scsi hostdev */
>> + int readonly;
>
> This should be a member of virDomainHostdevDef instead.
>
>> };
>>
>> enum virDomainSeclabelType {
>> @@ -1951,8 +1953,8 @@ void virDomainInputDefFree(virDomainInputDefPtr def);
>> void virDomainDiskDefFree(virDomainDiskDefPtr def);
>> void virDomainLeaseDefFree(virDomainLeaseDefPtr def);
>> void virDomainDiskHostDefFree(virDomainDiskHostDefPtr def);
>> -int virDomainDiskFindControllerModel(virDomainDefPtr def,
>> - virDomainDiskDefPtr disk,
>> +int virDomainInfoFindControllerModel(virDomainDefPtr def,
>> + virDomainDeviceInfoPtr info,
>> int controllerType);
>> virDomainDiskDefPtr virDomainDiskFindByBusAndDst(virDomainDefPtr def,
>> int bus,
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index c7bd847..4fc6b32 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -143,7 +143,6 @@ virDomainDiskDeviceTypeToString;
>> virDomainDiskErrorPolicyTypeFromString;
>> virDomainDiskErrorPolicyTypeToString;
>> virDomainDiskFindByBusAndDst;
>> -virDomainDiskFindControllerModel;
>> virDomainDiskGeometryTransTypeFromString;
>> virDomainDiskGeometryTransTypeToString;
>> virDomainDiskHostDefFree;
>> @@ -213,6 +212,7 @@ virDomainHubTypeFromString;
>> virDomainHubTypeToString;
>> virDomainHypervTypeFromString;
>> virDomainHypervTypeToString;
>> +virDomainInfoFindControllerModel;
>> virDomainInputDefFree;
>> virDomainIoEventFdTypeFromString;
>> virDomainIoEventFdTypeToString;
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 201fac1..5eb9999 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -549,7 +549,7 @@ qemuAssignDeviceDiskAliasCustom(virDomainDefPtr def,
>> if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
>> if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
>> controllerModel =
>> - virDomainDiskFindControllerModel(def, disk,
>> + virDomainInfoFindControllerModel(def,&disk->info,
>> VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
>>
>> if ((qemuSetScsiControllerModel(def, qemuCaps,&controllerModel))< 0)
>> @@ -2699,7 +2699,7 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
>> }
>>
>> controllerModel =
>> - virDomainDiskFindControllerModel(def, disk,
>> + virDomainInfoFindControllerModel(def,&disk->info,
>> VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
>> if ((qemuSetScsiControllerModel(def, qemuCaps,&controllerModel))< 0)
>> goto error;
>
> You need new tests for the new XML.
>
More information about the libvir-list
mailing list