[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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. 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]