12:24 p.m.
The recently added option to control the disk format probing has broken
AppArmor related code and tests. This series fixes this.
Matthias
12:24 p.m.
New subject: [libvirt] [PATCH 1/5] virt-aa-helper: Ignore open errors again
virt-aa-helper used to ignore errors when opening files.
Commit a8853344994a7c6aaca882a5e949ab5536821ab5 refactored
the related code and changed this behavior. virt-aa-helper
didn't ignore open errors anymore and virt-aa-helper-test
fails.
Make sure that virt-aa-helper ignores open errors again.
---
src/security/virt-aa-helper.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 521545d..16b1920 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -846,7 +846,7 @@ get_files(vahControl * ctl)
for (i = 0; i < ctl->def->ndisks; i++) {
int ret = virDomainDiskDefForeachPath(ctl->def->disks[i],
ctl->allowDiskFormatProbing,
- false,
+ true,
add_file_path,
&buf);
if (ret != 0)
--
1.7.0.4
5:28 p.m.
New subject: [libvirt] [PATCH 1/5] virt-aa-helper: Ignore open errors again
On Fri, 2010-07-23 at 19:24 +0200, Matthias Bolte wrote:
virt-aa-helper used to ignore errors when opening files.
Commit a8853344994a7c6aaca882a5e949ab5536821ab5 refactored
the related code and changed this behavior. virt-aa-helper
didn't ignore open errors anymore and virt-aa-helper-test
fails.
Make sure that virt-aa-helper ignores open errors again.
---
src/security/virt-aa-helper.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 521545d..16b1920 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -846,7 +846,7 @@ get_files(vahControl * ctl)
for (i = 0; i < ctl->def->ndisks; i++) {
int ret = virDomainDiskDefForeachPath(ctl->def->disks[i],
ctl->allowDiskFormatProbing,
- false,
+ true,
add_file_path,
&buf);
if (ret != 0)
I'm not 100% sure on this one. I have been developing patches to adjust
for the new behavior on older releases and I did some shuffling to get
this to work with 'false'. I'm not ready to submit at this time, and
won't be able to get to it until the week after next. If this blocks
Matthias' work, then feel free to commit and I'll post with a different
patch if needed. Otherwise, we can wait.
--
Jamie Strandboge | http://www.canonical.com
5:49 p.m.
New subject: [libvirt] [PATCH 1/5] virt-aa-helper: Ignore open errors again
2010/7/24 Jamie Strandboge <jamie(a)canonical.com>:
On Fri, 2010-07-23 at 19:24 +0200, Matthias Bolte wrote:
> virt-aa-helper used to ignore errors when opening files.
> Commit a8853344994a7c6aaca882a5e949ab5536821ab5 refactored
> the related code and changed this behavior. virt-aa-helper
> didn't ignore open errors anymore and virt-aa-helper-test
> fails.
>
> Make sure that virt-aa-helper ignores open errors again.
> ---
> src/security/virt-aa-helper.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index 521545d..16b1920 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -846,7 +846,7 @@ get_files(vahControl * ctl)
> for (i = 0; i < ctl->def->ndisks; i++) {
> int ret = virDomainDiskDefForeachPath(ctl->def->disks[i],
> ctl->allowDiskFormatProbing,
> - false,
> + true,
> add_file_path,
> &buf);
> if (ret != 0)
I'm not 100% sure on this one. I have been developing patches to adjust
for the new behavior on older releases and I did some shuffling to get
this to work with 'false'. I'm not ready to submit at this time, and
won't be able to get to it until the week after next. If this blocks
Matthias' work, then feel free to commit and I'll post with a different
patch if needed. Otherwise, we can wait.
--
Jamie Strandboge | http://www.canonical.com
It doesn't block my work directly, it just makes make check fail (if
you have AppArmor installed), because the refactoring changed the
original behavior.
As Daniel plans the release of 0.8.3 for next Friday and you won't be
able to work on your patches before the release, I'd like to push this
patch in order to have a clean make check in 0.8.3.
Once your patches to improve virt-aa-helper are finished we can go
back to 'false'.
Matthias
5:57 p.m.
New subject: [libvirt] [PATCH 1/5] virt-aa-helper: Ignore open errors again
On 07/23/2010 04:49 PM, Matthias Bolte wrote:
It doesn't block my work directly, it just makes make check fail
(if
you have AppArmor installed), because the refactoring changed the
original behavior.
As Daniel plans the release of 0.8.3 for next Friday and you won't be
able to work on your patches before the release, I'd like to push this
patch in order to have a clean make check in 0.8.3.
Once your patches to improve virt-aa-helper are finished we can go
back to 'false'.
Agreed to that plan; but to help us remember, add a XXX comment to
remind us that we intend to change back later.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
3:52 a.m.
New subject: [libvirt] [PATCH 1/5] virt-aa-helper: Ignore open errors again
On Fri, Jul 23, 2010 at 04:57:32PM -0600, Eric Blake wrote:
On 07/23/2010 04:49 PM, Matthias Bolte wrote:
>It doesn't block my work directly, it just makes make check fail (if
>you have AppArmor installed), because the refactoring changed the
>original behavior.
>
>As Daniel plans the release of 0.8.3 for next Friday and you won't be
>able to work on your patches before the release, I'd like to push this
>patch in order to have a clean make check in 0.8.3.
>
>Once your patches to improve virt-aa-helper are finished we can go
>back to 'false'.
Agreed to that plan; but to help us remember, add a XXX comment to
remind us that we intend to change back later.
Yep, that sounds the best way :-)
ACK
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
10:28 a.m.
New subject: [libvirt] [PATCH 1/5] virt-aa-helper: Ignore open errors again
2010/7/24 Daniel Veillard <veillard(a)redhat.com>:
On Fri, Jul 23, 2010 at 04:57:32PM -0600, Eric Blake wrote:
> On 07/23/2010 04:49 PM, Matthias Bolte wrote:
> >It doesn't block my work directly, it just makes make check fail (if
> >you have AppArmor installed), because the refactoring changed the
> >original behavior.
> >
> >As Daniel plans the release of 0.8.3 for next Friday and you won't be
> >able to work on your patches before the release, I'd like to push this
> >patch in order to have a clean make check in 0.8.3.
> >
> >Once your patches to improve virt-aa-helper are finished we can go
> >back to 'false'.
>
> Agreed to that plan; but to help us remember, add a XXX comment to
> remind us that we intend to change back later.
Yep, that sounds the best way :-)
ACK
Daniel
Okay, I added the comment suggest by Eric and pushed the result.
Matthias
4:49 a.m.
New subject: [libvirt] [PATCH 1/5] virt-aa-helper: Ignore open errors again
On Sat, Jul 24, 2010 at 12:28:11AM +0200, Jamie Strandboge wrote:
On Fri, 2010-07-23 at 19:24 +0200, Matthias Bolte wrote:
> virt-aa-helper used to ignore errors when opening files.
> Commit a8853344994a7c6aaca882a5e949ab5536821ab5 refactored
> the related code and changed this behavior. virt-aa-helper
> didn't ignore open errors anymore and virt-aa-helper-test
> fails.
>
> Make sure that virt-aa-helper ignores open errors again.
> ---
> src/security/virt-aa-helper.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index 521545d..16b1920 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -846,7 +846,7 @@ get_files(vahControl * ctl)
> for (i = 0; i < ctl->def->ndisks; i++) {
> int ret = virDomainDiskDefForeachPath(ctl->def->disks[i],
> ctl->allowDiskFormatProbing,
> - false,
> + true,
> add_file_path,
> &buf);
> if (ret != 0)
I'm not 100% sure on this one. I have been developing patches to adjust
for the new behavior on older releases and I did some shuffling to get
this to work with 'false'. I'm not ready to submit at this time, and
won't be able to get to it until the week after next. If this blocks
Matthias' work, then feel free to commit and I'll post with a different
patch if needed. Otherwise, we can wait.
What is the scenario in which 'false' breaks things ? We use 'false' for
the selinux driver already. The problem with 'true' is that it means the
user will never see potentially important errors.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
4:46 a.m.
New subject: [libvirt] [PATCH 1/5] virt-aa-helper: Ignore open errors again
2010/7/26 Daniel P. Berrange <berrange(a)redhat.com>:
On Sat, Jul 24, 2010 at 12:28:11AM +0200, Jamie Strandboge wrote:
> On Fri, 2010-07-23 at 19:24 +0200, Matthias Bolte wrote:
> > virt-aa-helper used to ignore errors when opening files.
> > Commit a8853344994a7c6aaca882a5e949ab5536821ab5 refactored
> > the related code and changed this behavior. virt-aa-helper
> > didn't ignore open errors anymore and virt-aa-helper-test
> > fails.
> >
> > Make sure that virt-aa-helper ignores open errors again.
> > ---
> > src/security/virt-aa-helper.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> > index 521545d..16b1920 100644
> > --- a/src/security/virt-aa-helper.c
> > +++ b/src/security/virt-aa-helper.c
> > @@ -846,7 +846,7 @@ get_files(vahControl * ctl)
> > for (i = 0; i < ctl->def->ndisks; i++) {
> > int ret = virDomainDiskDefForeachPath(ctl->def->disks[i],
> > ctl->allowDiskFormatProbing,
> > - false,
> > + true,
> > add_file_path,
> > &buf);
> > if (ret != 0)
>
> I'm not 100% sure on this one. I have been developing patches to adjust
> for the new behavior on older releases and I did some shuffling to get
> this to work with 'false'. I'm not ready to submit at this time, and
> won't be able to get to it until the week after next. If this blocks
> Matthias' work, then feel free to commit and I'll post with a different
> patch if needed. Otherwise, we can wait.
What is the scenario in which 'false' breaks things ? We use 'false' for
the selinux driver already. The problem with 'true' is that it means the
user will never see potentially important errors.
Regards,
Daniel
Maybe it's a problem that's triggered by the test only.
Passing 'false' makes virt-aa-helper-test fail for tests that involve
non-existing files:
$ ./virt-aa-helper-test
FAIL: exited with '1'
create (non-existent disk): '--dryrun -c -u
libvirt-00000000-0000-0000-0000-0123456789ab':
FAIL: exited with '1'
replace (non-existent disk): '--dryrun -r -u
libvirt-00000000-0000-0000-0000-0123456789ab':
Here's the command that virt-aa-helper-test executes for the first failing test:
$ LD_LIBRARY_PATH="../src/.libs/" ../src/virt-aa-helper --dryrun -c -u
libvirt-00000000-0000-0000-0000-0123456789ab <
/tmp/tmp.IFQ5dqTvp6/test.xml
virt-aa-helper: warning: path does not exist, skipping file type checks
virt-aa-helper: error: invalid VM definition
Here's the content of /tmp/tmp.IFQ5dqTvp6/test.xml
<domain type='kvm'>
<name>virt-aa-helper-test</name>
<uuid>00000000-0000-0000-0000-0123456789ab</uuid>
<memory>524288</memory>
<currentMemory>524288</currentMemory>
<vcpu>1</vcpu>
<os>
<type arch='x86_64' machine='pc'>hvm</type>
<boot dev='hd'/>
</os>
<features>
<acpi/>
</features>
<clock offset='utc'/>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
<on_crash>destroy</on_crash>
<devices>
<emulator>/usr/bin/kvm</emulator>
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/tmp/tmp.IFQ5dqTvp6/nonexistant.img'/>
<target dev='hda' bus='ide'/>
</disk>
<interface type='network'>
<mac address='52:54:00:50:4b:26'/>
<source network='default'/>
<model type='virtio'/>
</interface>
<input type='tablet' bus='usb'/>
<input type='mouse' bus='ps2'/>
<graphics type='vnc' port='-1' autoport='yes'
listen='127.0.0.1'/>
<video>
<model type='cirrus' vram='9216' heads='1'/>
</video>
</devices>
</domain>
I noticed that this failed because the rewrite to use
virDomainDiskDefForeachPath changed the handling of non-existing
files. So I changed it back. That might not be the proper solution,
but it works. I seems that Jamie is working on a better solution for
that case.
Matthias
4:51 a.m.
New subject: [libvirt] [PATCH 1/5] virt-aa-helper: Ignore open errors again
On Tue, Jul 27, 2010 at 11:46:34AM +0200, Matthias Bolte wrote:
2010/7/26 Daniel P. Berrange <berrange(a)redhat.com>:
> On Sat, Jul 24, 2010 at 12:28:11AM +0200, Jamie Strandboge wrote:
>> On Fri, 2010-07-23 at 19:24 +0200, Matthias Bolte wrote:
>> > virt-aa-helper used to ignore errors when opening files.
>> > Commit a8853344994a7c6aaca882a5e949ab5536821ab5 refactored
>> > the related code and changed this behavior. virt-aa-helper
>> > didn't ignore open errors anymore and virt-aa-helper-test
>> > fails.
>> >
>> > Make sure that virt-aa-helper ignores open errors again.
>> > ---
>> > src/security/virt-aa-helper.c | 2 +-
>> > 1 files changed, 1 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
>> > index 521545d..16b1920 100644
>> > --- a/src/security/virt-aa-helper.c
>> > +++ b/src/security/virt-aa-helper.c
>> > @@ -846,7 +846,7 @@ get_files(vahControl * ctl)
>> > for (i = 0; i < ctl->def->ndisks; i++) {
>> > int ret = virDomainDiskDefForeachPath(ctl->def->disks[i],
>> >
ctl->allowDiskFormatProbing,
>> > - false,
>> > + true,
>> > add_file_path,
>> > &buf);
>> > if (ret != 0)
>>
>> I'm not 100% sure on this one. I have been developing patches to adjust
>> for the new behavior on older releases and I did some shuffling to get
>> this to work with 'false'. I'm not ready to submit at this time,
and
>> won't be able to get to it until the week after next. If this blocks
>> Matthias' work, then feel free to commit and I'll post with a different
>> patch if needed. Otherwise, we can wait.
>
> What is the scenario in which 'false' breaks things ? We use 'false'
for
> the selinux driver already. The problem with 'true' is that it means the
> user will never see potentially important errors.
>
> Regards,
> Daniel
Maybe it's a problem that's triggered by the test only.
Passing 'false' makes virt-aa-helper-test fail for tests that involve
non-existing files:
$ ./virt-aa-helper-test
FAIL: exited with '1'
create (non-existent disk): '--dryrun -c -u
libvirt-00000000-0000-0000-0000-0123456789ab':
FAIL: exited with '1'
replace (non-existent disk): '--dryrun -r -u
libvirt-00000000-0000-0000-0000-0123456789ab':
Here's the command that virt-aa-helper-test executes for the first failing test:
$ LD_LIBRARY_PATH="../src/.libs/" ../src/virt-aa-helper --dryrun -c -u
libvirt-00000000-0000-0000-0000-0123456789ab <
/tmp/tmp.IFQ5dqTvp6/test.xml
virt-aa-helper: warning: path does not exist, skipping file type checks
virt-aa-helper: error: invalid VM definition
Hmm, this does look like a test script bug. It should at least touch
the files so they exist before running. If a real deployment you would
actually want these kind of error messages about non-existant files.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
5:49 a.m.
New subject: [libvirt] [PATCH 1/5] virt-aa-helper: Ignore open errors again
2010/7/27 Daniel P. Berrange <berrange(a)redhat.com>:
On Tue, Jul 27, 2010 at 11:46:34AM +0200, Matthias Bolte wrote:
> 2010/7/26 Daniel P. Berrange <berrange(a)redhat.com>:
> > On Sat, Jul 24, 2010 at 12:28:11AM +0200, Jamie Strandboge wrote:
> >> On Fri, 2010-07-23 at 19:24 +0200, Matthias Bolte wrote:
> >> > virt-aa-helper used to ignore errors when opening files.
> >> > Commit a8853344994a7c6aaca882a5e949ab5536821ab5 refactored
> >> > the related code and changed this behavior. virt-aa-helper
> >> > didn't ignore open errors anymore and virt-aa-helper-test
> >> > fails.
> >> >
> >> > Make sure that virt-aa-helper ignores open errors again.
> >> > ---
> >> > src/security/virt-aa-helper.c | 2 +-
> >> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >> >
> >> > diff --git a/src/security/virt-aa-helper.c
b/src/security/virt-aa-helper.c
> >> > index 521545d..16b1920 100644
> >> > --- a/src/security/virt-aa-helper.c
> >> > +++ b/src/security/virt-aa-helper.c
> >> > @@ -846,7 +846,7 @@ get_files(vahControl * ctl)
> >> > for (i = 0; i < ctl->def->ndisks; i++) {
> >> > int ret =
virDomainDiskDefForeachPath(ctl->def->disks[i],
> >> >
ctl->allowDiskFormatProbing,
> >> > - false,
> >> > + true,
> >> > add_file_path,
> >> > &buf);
> >> > if (ret != 0)
> >>
> >> I'm not 100% sure on this one. I have been developing patches to adjust
> >> for the new behavior on older releases and I did some shuffling to get
> >> this to work with 'false'. I'm not ready to submit at this time,
and
> >> won't be able to get to it until the week after next. If this blocks
> >> Matthias' work, then feel free to commit and I'll post with a
different
> >> patch if needed. Otherwise, we can wait.
> >
> > What is the scenario in which 'false' breaks things ? We use
'false' for
> > the selinux driver already. The problem with 'true' is that it means
the
> > user will never see potentially important errors.
> >
> > Regards,
> > Daniel
>
> Maybe it's a problem that's triggered by the test only.
>
> Passing 'false' makes virt-aa-helper-test fail for tests that involve
> non-existing files:
>
> $ ./virt-aa-helper-test
> FAIL: exited with '1'
> create (non-existent disk): '--dryrun -c -u
> libvirt-00000000-0000-0000-0000-0123456789ab':
> FAIL: exited with '1'
> replace (non-existent disk): '--dryrun -r -u
> libvirt-00000000-0000-0000-0000-0123456789ab':
>
> Here's the command that virt-aa-helper-test executes for the first failing test:
>
> $ LD_LIBRARY_PATH="../src/.libs/" ../src/virt-aa-helper --dryrun -c -u
> libvirt-00000000-0000-0000-0000-0123456789ab <
> /tmp/tmp.IFQ5dqTvp6/test.xml
> virt-aa-helper: warning: path does not exist, skipping file type checks
> virt-aa-helper: error: invalid VM definition
Hmm, this does look like a test script bug. It should at least touch
the files so they exist before running. If a real deployment you would
actually want these kind of error messages about non-existant files.
I don't think that it's a bug in the test script in the way that it
should touch the files. The test are named "create (non-existent
disk)" and "replace (non-existent disk)". I think the are meant to
work with non-existent files.
As I think of it, I'm not sure whether or not this two tests cases
should be there at all.
Is there a real use case with non-existent files that this two test
cover? If not then we could back to 'true' and remove this two test
cases.
Matthias
5:52 a.m.
New subject: [libvirt] [PATCH 1/5] virt-aa-helper: Ignore open errors again
On Tue, Jul 27, 2010 at 12:49:45PM +0200, Matthias Bolte wrote:
2010/7/27 Daniel P. Berrange <berrange(a)redhat.com>:
> On Tue, Jul 27, 2010 at 11:46:34AM +0200, Matthias Bolte wrote:
>> 2010/7/26 Daniel P. Berrange <berrange(a)redhat.com>:
>> > On Sat, Jul 24, 2010 at 12:28:11AM +0200, Jamie Strandboge wrote:
>> >> On Fri, 2010-07-23 at 19:24 +0200, Matthias Bolte wrote:
>> >> > virt-aa-helper used to ignore errors when opening files.
>> >> > Commit a8853344994a7c6aaca882a5e949ab5536821ab5 refactored
>> >> > the related code and changed this behavior. virt-aa-helper
>> >> > didn't ignore open errors anymore and virt-aa-helper-test
>> >> > fails.
>> >> >
>> >> > Make sure that virt-aa-helper ignores open errors again.
>> >> > ---
>> >> > src/security/virt-aa-helper.c | 2 +-
>> >> > 1 files changed, 1 insertions(+), 1 deletions(-)
>> >> >
>> >> > diff --git a/src/security/virt-aa-helper.c
b/src/security/virt-aa-helper.c
>> >> > index 521545d..16b1920 100644
>> >> > --- a/src/security/virt-aa-helper.c
>> >> > +++ b/src/security/virt-aa-helper.c
>> >> > @@ -846,7 +846,7 @@ get_files(vahControl * ctl)
>> >> > for (i = 0; i < ctl->def->ndisks; i++) {
>> >> > int ret =
virDomainDiskDefForeachPath(ctl->def->disks[i],
>> >> >
ctl->allowDiskFormatProbing,
>> >> > - false,
>> >> > + true,
>> >> > add_file_path,
>> >> > &buf);
>> >> > if (ret != 0)
>> >>
>> >> I'm not 100% sure on this one. I have been developing patches to
adjust
>> >> for the new behavior on older releases and I did some shuffling to get
>> >> this to work with 'false'. I'm not ready to submit at this
time, and
>> >> won't be able to get to it until the week after next. If this
blocks
>> >> Matthias' work, then feel free to commit and I'll post with a
different
>> >> patch if needed. Otherwise, we can wait.
>> >
>> > What is the scenario in which 'false' breaks things ? We use
'false' for
>> > the selinux driver already. The problem with 'true' is that it
means the
>> > user will never see potentially important errors.
>> >
>> > Regards,
>> > Daniel
>>
>> Maybe it's a problem that's triggered by the test only.
>>
>> Passing 'false' makes virt-aa-helper-test fail for tests that involve
>> non-existing files:
>>
>> $ ./virt-aa-helper-test
>> FAIL: exited with '1'
>> create (non-existent disk): '--dryrun -c -u
>> libvirt-00000000-0000-0000-0000-0123456789ab':
>> FAIL: exited with '1'
>> replace (non-existent disk): '--dryrun -r -u
>> libvirt-00000000-0000-0000-0000-0123456789ab':
>>
>> Here's the command that virt-aa-helper-test executes for the first failing
test:
>>
>> $ LD_LIBRARY_PATH="../src/.libs/" ../src/virt-aa-helper --dryrun -c
-u
>> libvirt-00000000-0000-0000-0000-0123456789ab <
>> /tmp/tmp.IFQ5dqTvp6/test.xml
>> virt-aa-helper: warning: path does not exist, skipping file type checks
>> virt-aa-helper: error: invalid VM definition
>
> Hmm, this does look like a test script bug. It should at least touch
> the files so they exist before running. If a real deployment you would
> actually want these kind of error messages about non-existant files.
>
I don't think that it's a bug in the test script in the way that it
should touch the files. The test are named "create (non-existent
disk)" and "replace (non-existent disk)". I think the are meant to
work with non-existent files.
As I think of it, I'm not sure whether or not this two tests cases
should be there at all.
Is there a real use case with non-existent files that this two test
cover? If not then we could back to 'true' and remove this two test
cases.
Perhaps the tests should be changed to verify that it failed, since I
think that is the desired behaviour with non-existant files
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
12:24 p.m.
New subject: [libvirt] [PATCH 2/5] virt-aa-helper: Fix return value of add_file_path
Commit a8853344994a7c6aaca882a5e949ab5536821ab5 added this
function and wrapped vah_add_file in it. vah_add_file may
return -1, 0, 1. It returns 1 in case the call to valid_path
detects a restricted file. The original code treated a return
value != 0 as error. The refactored code treats a return
value < 0 as error. This triggers segfault in virt-aa-helper
and breaks virt-aa-helper-test for the restricted file tests.
Make sure that add_file_path returns -1 on error.
---
src/security/virt-aa-helper.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 16b1920..da7d515 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -818,6 +818,9 @@ add_file_path(virDomainDiskDefPtr disk,
ret = vah_add_file(buf, path, "r");
}
+ if (ret != 0)
+ ret = -1;
+
return ret;
}
--
1.7.0.4
5:19 p.m.
New subject: [libvirt] [PATCH 2/5] virt-aa-helper: Fix return value of add_file_path
On Fri, 2010-07-23 at 19:24 +0200, Matthias Bolte wrote:
Commit a8853344994a7c6aaca882a5e949ab5536821ab5 added this
function and wrapped vah_add_file in it. vah_add_file may
return -1, 0, 1. It returns 1 in case the call to valid_path
detects a restricted file. The original code treated a return
value != 0 as error. The refactored code treats a return
value < 0 as error. This triggers segfault in virt-aa-helper
and breaks virt-aa-helper-test for the restricted file tests.
Make sure that add_file_path returns -1 on error.
ACK
--
Jamie Strandboge | http://www.canonical.com
12:24 p.m.
New subject: [libvirt] [PATCH 3/5] virt-aa-helper: Make getopt accept the p option
Commit 68719c4bddb85fbcc931a5b7d99ac7c8a0af09b0 added the
p option to control disk format probing, but it wasn't added
to the getopt_long optstring parameter.
Add the p option to the getopt_long optstring parameter.
---
src/security/virt-aa-helper.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index da7d515..44d1a6d 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -960,7 +960,7 @@ vahParseArgv(vahControl * ctl, int argc, char **argv)
{0, 0, 0, 0}
};
- while ((arg = getopt_long(argc, argv, "acdDhrRH:b:u:f:", opt,
+ while ((arg = getopt_long(argc, argv, "acdDhrRH:b:u:p:f:", opt,
&idx)) != -1) {
switch (arg) {
case 'a':
--
1.7.0.4
5:20 p.m.
New subject: [libvirt] [PATCH 3/5] virt-aa-helper: Make getopt accept the p option
On Fri, 2010-07-23 at 19:24 +0200, Matthias Bolte wrote:
Commit 68719c4bddb85fbcc931a5b7d99ac7c8a0af09b0 added the
p option to control disk format probing, but it wasn't added
to the getopt_long optstring parameter.
Add the p option to the getopt_long optstring parameter.
ACK
--
Jamie Strandboge | http://www.canonical.com
12:24 p.m.
New subject: [libvirt] [PATCH 4/5] virt-aa-helper-test: Fix failure due to the new disk format probing option
Commit 68719c4bddb85fbcc931a5b7d99ac7c8a0af09b0 added the disk format
probing option. This makes virt-aa-helper-test fail because the domain
config didn't specifiy the disk format and it didn't pass '-p 1' to
virt-aa-helper to allow disk format probing.
Specify the disk format in the domain config. Pass the '-p 1' option
to virt-aa-helper for the test case with two disks. This way this test
also covers this new option.
---
tests/virt-aa-helper-test | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test
index ada89f4..6c97aaf 100755
--- a/tests/virt-aa-helper-test
+++ b/tests/virt-aa-helper-test
@@ -76,6 +76,7 @@ cat > "$template_xml" <<EOM
<devices>
<emulator>/usr/bin/kvm</emulator>
<disk type='file' device='disk'>
+ <driver name='qemu' type='raw'/>
<source file='###DISK###'/>
<target dev='hda' bus='ide'/>
</disk>
@@ -195,7 +196,7 @@ cat "$template_xml" | sed "s,###UUID###,$uuid,g" |
sed "s,###DISK###,$disk1,g" |
testme "0" "create (ppc)" "-c -u $valid_uuid"
"$test_xml"
cat "$template_xml" | sed "s,###UUID###,$uuid,g" | sed
"s,###DISK###,$disk1,g" | sed "s,</disk>,</disk><disk
type='file' device='disk'><source
file='$disk2'/><target dev='hdb'
bus='ide'/></disk>,g" > "$test_xml"
-testme "0" "create multiple disks" "-c -u $valid_uuid"
"$test_xml"
+testme "0" "create multiple disks" "-c -u $valid_uuid -p 1"
"$test_xml"
cat "$template_xml" | sed "s,###UUID###,$uuid,g" | sed
"s,###DISK###',${disk1}'/><readonly,g" > "$test_xml"
testme "0" "create (readonly)" "-c -u $valid_uuid"
"$test_xml"
--
1.7.0.4
5:22 p.m.
New subject: [libvirt] [PATCH 4/5] virt-aa-helper-test: Fix failure due to the new disk format probing option
On Fri, 2010-07-23 at 19:24 +0200, Matthias Bolte wrote:
Commit 68719c4bddb85fbcc931a5b7d99ac7c8a0af09b0 added the disk
format
probing option. This makes virt-aa-helper-test fail because the domain
config didn't specifiy the disk format and it didn't pass '-p 1' to
virt-aa-helper to allow disk format probing.
Specify the disk format in the domain config. Pass the '-p 1' option
to virt-aa-helper for the test case with two disks. This way this test
also covers this new option.
ACK
--
Jamie Strandboge | http://www.canonical.com
12:24 p.m.
New subject: [libvirt] [PATCH 5/5] secaatest: Fix compilation
Since 68719c4bddb85fbcc931a5b7d99ac7c8a0af09b0 virSecurityDriverStartup
takes and additional parameter to control disk format probing.
Pass false as third parameter.
---
tests/secaatest.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tests/secaatest.c b/tests/secaatest.c
index 54fdb44..d9d6b4a 100644
--- a/tests/secaatest.c
+++ b/tests/secaatest.c
@@ -15,7 +15,7 @@ main (int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED)
const char *doi, *model;
virSecurityDriverPtr security_drv;
- ret = virSecurityDriverStartup (&security_drv, "apparmor");
+ ret = virSecurityDriverStartup (&security_drv, "apparmor", false);
if (ret == -1)
{
fprintf (stderr, "Failed to start security driver");
--
1.7.0.4
5:21 p.m.
New subject: [libvirt] [PATCH 5/5] secaatest: Fix compilation
On Fri, 2010-07-23 at 19:24 +0200, Matthias Bolte wrote:
Since 68719c4bddb85fbcc931a5b7d99ac7c8a0af09b0
virSecurityDriverStartup
takes and additional parameter to control disk format probing.
Pass false as third parameter.
ACK
--
Jamie Strandboge | http://www.canonical.com
2:31 p.m.
On 07/23/2010 11:24 AM, Matthias Bolte wrote:
The recently added option to control the disk format probing has
broken
AppArmor related code and tests. This series fixes this.
Matthias
ACK series - all 5 patches are pretty obvious and self-contained.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
3:55 a.m.
On Fri, Jul 23, 2010 at 01:31:17PM -0600, Eric Blake wrote:
On 07/23/2010 11:24 AM, Matthias Bolte wrote:
>The recently added option to control the disk format probing has broken
>AppArmor related code and tests. This series fixes this.
>
>Matthias
ACK series - all 5 patches are pretty obvious and self-contained.
ACK too
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
10:29 a.m.
2010/7/24 Daniel Veillard <veillard(a)redhat.com>:
On Fri, Jul 23, 2010 at 01:31:17PM -0600, Eric Blake wrote:
> On 07/23/2010 11:24 AM, Matthias Bolte wrote:
> >The recently added option to control the disk format probing has broken
> >AppArmor related code and tests. This series fixes this.
> >
> >Matthias
>
> ACK series - all 5 patches are pretty obvious and self-contained.
ACK too
Daniel
Thanks, pushed the series.
Matthias
5232
days inactive
5236
days old
22 comments
5 participants
participants (5)
-
Daniel P. Berrange -
Daniel Veillard -
Eric Blake -
Jamie Strandboge -
Matthias Bolte