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

Re: [libvirt] [PATCH 2/2] qemu: don't relabel chardev source file if virtlogd is used



On Tue, May 23, 2017 at 05:13:17PM +0200, Erik Skultety wrote:
>     while (cur != NULL) {
> @@ -10628,6 +10630,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
>                 case VIR_DOMAIN_CHR_TYPE_UNIX:
>                     if (!append && def->type == VIR_DOMAIN_CHR_TYPE_FILE)
>                         append = virXMLPropString(cur, "append");
> +                    if (!skipRelabel && def->type == VIR_DOMAIN_CHR_TYPE_FILE)
> +                        skipRelabel = virXMLPropString(cur, "skipRelabel");

I'm guessing you want to keep this away from users, right?  You should
not be parsing it from the XML then.  Or you should add a thing there
that the XML supports.  Not just a random attribute.

Either keep this data in private structure or even better, just add the
same thing as you would do with:

 <seclabel relabel="no"/>

What sense would it make in this case? There is only one option when we're
running virtlogd and that is no label. If they supply relabel='yes', it won't
work with virtlogd unless they change it back to 'no'. So, you've got an option
the functionality of which depends on a service running, which is by default on
by the way.


If that's so clear, why put it in the XML?  Why not just set it in the
private structure then?  Either there is a reason for it to be variable
or not.  If there is not, then we're effectively talking about something
like <domain doNotFailOnStart='yes'> attribute.  There is no need for
such thing in that case.

Sure, I might be wrong.  In that case, this commit needs more explanation.

Since I don't want to be someone who just criticizes other people's ideas
without adding anything to the discussion, I was thinking about re-using the
qemu config parameter stdioLogD. But that solution is also wrong as it turned
out. I haven't come up with anything better yet, though. So, having something in
the status XML seems like a plausible approach to me.

Erik


with all the details of course.  The user can see it, can supply it, old
releases support it, all the stuff is there already.

I'm open to suggestions, but NACK to random "wannabe private" attributes.



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

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

Attachment: signature.asc
Description: Digital signature


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