On Tue, May 23, 2017 at 04:25:01PM +0200, Martin Kletzander wrote: > On Mon, May 15, 2017 at 04:28:35PM +0200, Pavel Hrdina wrote: > >If libvirt uses virtlogd instead of passing the file path directly > >to QEMU we shouldn't relabel the chardev source file, otherwise > >virtlogd will get a permission denied while reloading. > > > >Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=143098 > > > > You missed a digit and I'm too lazy to check 10 bugs for a reproducer ;) Yep, there should by additional 8 :) > >Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx> > >--- > > src/conf/domain_conf.c | 20 ++++++++++++++++++++ > > src/conf/domain_conf.h | 1 + > > src/qemu/qemu_command.c | 12 ++++++++---- > > src/security/security_dac.c | 6 ++++++ > > src/security/security_selinux.c | 6 ++++++ > > 5 files changed, 41 insertions(+), 4 deletions(-) > > > > I see you are changing the parser code, but you are not changing the > Relax-NG schema, neither any test. > > >diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > >index aa441fae3c..92f011d3a4 100644 > >--- a/src/conf/domain_conf.c > >+++ b/src/conf/domain_conf.c > >@@ -2064,6 +2064,7 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest, > > } > > > > dest->type = src->type; > >+ dest->skipRelabel = src->skipRelabel; > > > > return 0; > > } > >@@ -10608,6 +10609,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, > > char *append = NULL; > > char *haveTLS = NULL; > > char *tlsFromConfig = NULL; > >+ char *skipRelabel = NULL; > > int remaining = 0; > > > > 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. We need to store it in the status XML so we don't lose the information while restarting libvirtd. And I want to keep it hidden from users because they shouldn't care about it. The decision whether we will use virtlogd as stdio handler is made by checking the qemu.conf for "stdio_handler" and whether QEMU supports appending to files. > Either keep this data in private structure or even better, just add the What do you mean by private structure? > same thing as you would do with: > > <seclabel relabel="no"/> > > with all the details of course. The user can see it, can supply it, old > releases support it, all the stuff is there already. This for user to specify it, not to store our internal private information that should survive restarting libvirtd. > I'm open to suggestions, but NACK to random "wannabe private" attributes. The only reason why I used the new private attribute is that we already do it for graphics device (fromConfig and autoGenerated) and for TCP character device (tlsFromConfig). I can store it in the status part of the status XML outside of the domain part, but that would require mapping it to the actual character device. Pavel
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
