9:29 a.m.
isos are read-only, so libvirt doesn't need to chown them. In one of
our testing setups, libvirt uses mirrorred isos. Since libvirt chowns
the files, (and especially does not chown them back) the mirror refuses
to update the iso.
This patch prevents libvirt from chowning files.
Does this seem reasonable?
Signed-off-by: Serge Hallyn <serge.hallyn(a)canonical.com>
---
src/security/security_dac.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index af02236..e7db324 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -555,6 +555,8 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr,
/* XXX fixme - we need to recursively label the entire tree :-( */
if (vm->def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_DIR)
continue;
+ if (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
+ continue;
if (virSecurityDACSetSecurityImageLabel(mgr,
vm,
vm->def->disks[i]) < 0)
--
1.7.5.4
12:49 p.m.
Quoting Serge E. Hallyn (serge.hallyn(a)canonical.com):
isos are read-only, so libvirt doesn't need to chown them. In
one of
our testing setups, libvirt uses mirrorred isos. Since libvirt chowns
the files, (and especially does not chown them back) the mirror refuses
to update the iso.
This patch prevents libvirt from chowning files.
Does this seem reasonable?
Hi,
any feedback on this? Does it seem ok?
thanks,
-serge
Signed-off-by: Serge Hallyn <serge.hallyn(a)canonical.com>
---
src/security/security_dac.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index af02236..e7db324 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -555,6 +555,8 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr,
/* XXX fixme - we need to recursively label the entire tree :-( */
if (vm->def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_DIR)
continue;
+ if (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
+ continue;
if (virSecurityDACSetSecurityImageLabel(mgr,
vm,
vm->def->disks[i]) < 0)
--
1.7.5.4
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
5:33 a.m.
On Tue, Oct 04, 2011 at 12:49:03PM -0500, Serge E. Hallyn wrote:
Quoting Serge E. Hallyn (serge.hallyn(a)canonical.com):
> isos are read-only, so libvirt doesn't need to chown them. In one of
> our testing setups, libvirt uses mirrorred isos. Since libvirt chowns
> the files, (and especially does not chown them back) the mirror refuses
> to update the iso.
>
> This patch prevents libvirt from chowning files.
>
> Does this seem reasonable?
any feedback on this? Does it seem ok?
Unfortunately while this would fix the use case you describe, it would
also break other use cases.
What we really need todo with the DAC driver is replace all the
chown() code, with code that sets ACLs instead. Well actually we
would need to keep the chown code as a fallback for filesystems
which don't support ACLs, but as long as we prefer ACLs by default
that'd be OK.
Of course when we have ACLs, we'd only need to grant 'r' to the
file for CDROMs which would be better than what we do now.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
5:44 a.m.
Quoting Daniel P. Berrange (berrange(a)redhat.com):
On Tue, Oct 04, 2011 at 12:49:03PM -0500, Serge E. Hallyn wrote:
> Quoting Serge E. Hallyn (serge.hallyn(a)canonical.com):
> > isos are read-only, so libvirt doesn't need to chown them. In one of
> > our testing setups, libvirt uses mirrorred isos. Since libvirt chowns
> > the files, (and especially does not chown them back) the mirror refuses
> > to update the iso.
> >
> > This patch prevents libvirt from chowning files.
> >
> > Does this seem reasonable?
>
> any feedback on this? Does it seem ok?
Unfortunately while this would fix the use case you describe, it would
also break other use cases.
What we really need todo with the DAC driver is replace all the
chown() code, with code that sets ACLs instead. Well actually we
would need to keep the chown code as a fallback for filesystems
which don't support ACLs, but as long as we prefer ACLs by default
that'd be OK.
Of course when we have ACLs, we'd only need to grant 'r' to the
file for CDROMs which would be better than what we do now.
Thanks, Daniel, that makes sense.
-serge
9:29 a.m.
On 10/05/2011 06:33 AM, Daniel P. Berrange wrote:
On Tue, Oct 04, 2011 at 12:49:03PM -0500, Serge E. Hallyn wrote:
> Quoting Serge E. Hallyn (serge.hallyn(a)canonical.com):
>> isos are read-only, so libvirt doesn't need to chown them. In one of
>> our testing setups, libvirt uses mirrorred isos. Since libvirt chowns
>> the files, (and especially does not chown them back) the mirror refuses
>> to update the iso.
>>
>> This patch prevents libvirt from chowning files.
>>
>> Does this seem reasonable?
> any feedback on this? Does it seem ok?
Unfortunately while this would fix the use case you describe, it would
also break other use cases.
What we really need todo with the DAC driver is replace all the
chown() code, with code that sets ACLs instead. Well actually we
would need to keep the chown code as a fallback for filesystems
which don't support ACLs, but as long as we prefer ACLs by default
that'd be OK.
Of course when we have ACLs, we'd only need to grant 'r' to the
file for CDROMs which would be better than what we do now.
In the meantime, I think you can solve the problem with your mirror by
mounting the share read-only. When the filesystem is read-only, libvirt
will attempt the chown/chgrp and fail, but notice the failure is due to
a r/o (or root-squash) filesystem, and ignore the failure.
9:39 a.m.
Quoting Laine Stump (laine(a)laine.org):
On 10/05/2011 06:33 AM, Daniel P. Berrange wrote:
>On Tue, Oct 04, 2011 at 12:49:03PM -0500, Serge E. Hallyn wrote:
>>Quoting Serge E. Hallyn (serge.hallyn(a)canonical.com):
>>>isos are read-only, so libvirt doesn't need to chown them. In one of
>>>our testing setups, libvirt uses mirrorred isos. Since libvirt chowns
>>>the files, (and especially does not chown them back) the mirror refuses
>>>to update the iso.
>>>
>>>This patch prevents libvirt from chowning files.
>>>
>>>Does this seem reasonable?
>>any feedback on this? Does it seem ok?
>Unfortunately while this would fix the use case you describe, it would
>also break other use cases.
>
>What we really need todo with the DAC driver is replace all the
>chown() code, with code that sets ACLs instead. Well actually we
>would need to keep the chown code as a fallback for filesystems
>which don't support ACLs, but as long as we prefer ACLs by default
>that'd be OK.
>
>Of course when we have ACLs, we'd only need to grant 'r' to the
>file for CDROMs which would be better than what we do now.
In the meantime, I think you can solve the problem with your mirror
by mounting the share read-only. When the filesystem is read-only,
libvirt will attempt the chown/chgrp and fail, but notice the
failure is due to a r/o (or root-squash) filesystem, and ignore the
failure.
Thanks for the suggestion, Laine. I suspect the dir they're using needs
to be writeable, but I'll ask if it is feasible to use a read-only bind
mount of the directory.
thanks,
-serge
4796
days inactive
4818
days old
5 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Laine Stump -
Serge E. Hallyn -
Serge Hallyn