Re: [libvirt] [PATCH RFC 1/5] conf: Introduce readonly to hostdev and change helper function

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.