Re: [PATCH 1/4] conf: introduce seclabels in shmem device element

[lhuang <lhuang@xxxxxxxxxx>]

Hi Marc-André

On 07/27/2015 11:42 PM, Marc-André Lureau wrote:
> Hi
>
> On Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang <lhuang@xxxxxxxxxx> wrote:
> > Introduce a new element in shmem device element, this
> > could help users to change the shm label to a specified
> > label.
> >
> > Signed-off-by: Luyao Huang <lhuang@xxxxxxxxxx>
> > ---
> >   docs/formatdomain.html.in     |  7 ++++++
> >   docs/schemas/domaincommon.rng |  3 +++
> >   src/conf/domain_conf.c        | 55 ++++++++++++++++++++++++++++++++++---------
> >   src/conf/domain_conf.h        |  5 ++++
> >   4 files changed, 59 insertions(+), 11 deletions(-)
> It would be better with a small test, checking parsing and format.

Oh, right, i forgot that, thanks for pointing out that, i will add them 
in next version.

> > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> > index d0c1741..e02c67c 100644
> > --- a/docs/formatdomain.html.in
> > +++ b/docs/formatdomain.html.in
> > @@ -6098,6 +6098,13 @@ qemu-kvm -net nic,model=? /dev/null
> >         vectors. The <code>ioeventd</code> attribute enables/disables (values
> >         "on"/"off", respectively) ioeventfd.
> >       </dd>
> > +    <dt><code>seclabel</code></dt>
> > +    <dd>
> > +      The  optional <code>seclabel</code> to override the way that labelling
> The "element may contain an" optional <code>...

Okay

> > +      is done on the shm object path or shm server path.  If this
> > +      element is not present, the <a href="#seclabel">security label is inherited
> > +      from the per-domain setting</a>.
> > +    </dd>
> >     </dl>
> >
> >       <h4><a name="elementsMemory">Memory devices</a></h4>
> > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> > index 1120003..f58e8de 100644
> > --- a/docs/schemas/domaincommon.rng
> > +++ b/docs/schemas/domaincommon.rng
> > @@ -3323,6 +3323,9 @@
> >               </optional>
> >             </element>
> >           </optional>
> > +        <zeroOrMore>
> > +          <ref name='devSeclabel'/>
> > +        </zeroOrMore>
> >           <optional>
> >             <ref name="address"/>
> >           </optional>
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 73ac537..cb3d72a 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -11261,6 +11261,8 @@ virDomainNVRAMDefParseXML(xmlNodePtr node,
> >   static virDomainShmemDefPtr
> >   virDomainShmemDefParseXML(xmlNodePtr node,
> >                             xmlXPathContextPtr ctxt,
> > +                          virSecurityLabelDefPtr* vmSeclabels,
> > +                          int nvmSeclabels,
> >                             unsigned int flags)
> >   {
> >       char *tmp = NULL;
> > @@ -11332,6 +11334,10 @@ virDomainShmemDefParseXML(xmlNodePtr node,
> >       if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
> >           goto cleanup;
> >
> > +    if (virSecurityDeviceLabelDefParseXML(&def->seclabels, &def->nseclabels,
> > +                                          vmSeclabels, nvmSeclabels,
> > +                                          ctxt, flags) < 0)
> > +        goto cleanup;
> >
> >       ret = def;
> >       def = NULL;
> > @@ -12457,7 +12463,11 @@ virDomainDeviceDefParse(const char *xmlStr,
> >               goto error;
> >           break;
> >       case VIR_DOMAIN_DEVICE_SHMEM:
> > -        if (!(dev->data.shmem = virDomainShmemDefParseXML(node, ctxt, flags)))
> > +        if (!(dev->data.shmem = virDomainShmemDefParseXML(node,
> > +                                                          ctxt,
> > +                                                          def->seclabels,
> > +                                                          def->nseclabels,
> > +                                                          flags)))
> >               goto error;
> >           break;
> >       case VIR_DOMAIN_DEVICE_TPM:
> > @@ -16136,7 +16146,8 @@ virDomainDefParseXML(xmlDocPtr xml,
> >       for (i = 0; i < n; i++) {
> >           virDomainShmemDefPtr shmem;
> >           ctxt->node = nodes[i];
> > -        shmem = virDomainShmemDefParseXML(nodes[i], ctxt, flags);
> > +        shmem = virDomainShmemDefParseXML(nodes[i], ctxt, def->seclabels,
> > +                                          def->nseclabels, flags);
> >           if (!shmem)
> >               goto error;
> >
> > @@ -20308,6 +20319,8 @@ virDomainShmemDefFormat(virBufferPtr buf,
> >                           virDomainShmemDefPtr def,
> >                           unsigned int flags)
> >   {
> > +    size_t n;
> > +
> >       virBufferEscapeString(buf, "<shmem name='%s'", def->name);
> >
> >       if (!def->size &&
> > @@ -20341,6 +20354,9 @@ virDomainShmemDefFormat(virBufferPtr buf,
> >           virBufferAddLit(buf, "/>\n");
> >       }
> >
> > +    for (n = 0; n < def->nseclabels; n++)
> > +        virSecurityDeviceLabelDefFormat(buf, def->seclabels[n], flags);
> > +
> >       if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
> >           return -1;
> >
> > @@ -23851,11 +23867,25 @@ virDomainObjListExport(virDomainObjListPtr domlist,
> >   }
> >
> >
> > +static virSecurityDeviceLabelDefPtr
> > +virDomainGetDeviceSecurityLabelDef(virSecurityDeviceLabelDefPtr *seclabels,
> > +                                   size_t nseclabels,
> > +                                   const char *model)
> > +{
> > +    size_t i;
> > +
> > +    for (i = 0; i < nseclabels; i++) {
> > +        if (STREQ_NULLABLE(seclabels[i]->model, model))
> > +            return seclabels[i];
> > +    }
> > +    return NULL;
> > +}
> > +
> > +
> >   virSecurityLabelDefPtr
> >   virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model)
> >   {
> >       size_t i;
> > -    virSecurityLabelDefPtr seclabel = NULL;
> >
> >       if (def == NULL || model == NULL)
> >           return NULL;
> > @@ -23866,24 +23896,27 @@ virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model)
> >           if (STREQ(def->seclabels[i]->model, model))
> >               return def->seclabels[i];
> >       }
> > -
> > -    return seclabel;
> > +    return NULL;
> This looks like a seperate cleanup.

Yes, i will split this in another patch.

Thanks a lot for your review.

Luyao

> >   }
> >
> >
> >   virSecurityDeviceLabelDefPtr
> >   virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model)
> >   {
> > -    size_t i;
> > +    if (def == NULL)
> > +        return NULL;
> > +
> > +    return virDomainGetDeviceSecurityLabelDef(def->seclabels, def->nseclabels, model);
> > +}
> >
> > +
> > +virSecurityDeviceLabelDefPtr
> > +virDomainShmemDefGetSecurityLabelDef(virDomainShmemDefPtr def, const char *model)
> > +{
> >       if (def == NULL)
> >           return NULL;
> >
> > -    for (i = 0; i < def->nseclabels; i++) {
> > -        if (STREQ_NULLABLE(def->seclabels[i]->model, model))
> > -            return def->seclabels[i];
> > -    }
> > -    return NULL;
> > +    return virDomainGetDeviceSecurityLabelDef(def->seclabels, def->nseclabels, model);
> >   }
> >
> >
> > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> > index 0fe6b1a..1a0475e 100644
> > --- a/src/conf/domain_conf.h
> > +++ b/src/conf/domain_conf.h
> > @@ -1608,6 +1608,8 @@ struct _virDomainShmemDef {
> >           unsigned vectors;
> >           virTristateSwitch ioeventfd;
> >       } msi;
> > +    size_t nseclabels;
> > +    virSecurityDeviceLabelDefPtr *seclabels;
> >       virDomainDeviceInfo info;
> >   };
> >
> > @@ -2943,6 +2945,9 @@ virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model);
> >   virSecurityDeviceLabelDefPtr
> >   virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model);
> >
> > +virSecurityDeviceLabelDefPtr
> > +virDomainShmemDefGetSecurityLabelDef(virDomainShmemDefPtr def, const char *model);
> > +
> >   typedef const char* (*virEventActionToStringFunc)(int type);
> >   typedef int (*virEventActionFromStringFunc)(const char *type);
> >
> > --
> > 1.8.3.1
> >
> > --
> > libvir-list mailing list
> > libvir-list@xxxxxxxxxx
> > https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list