[libvirt] [PATCH 0/5] Fix compile and test errors in AppArmor related code

The recently added option to control the disk format probing has broken AppArmor related code and tests. This series fixes this. Matthias

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

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

2010/7/24 Jamie Strandboge <jamie@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

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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

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@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

2010/7/24 Daniel Veillard <veillard@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

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 :|

2010/7/26 Daniel P. Berrange <berrange@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

On Tue, Jul 27, 2010 at 11:46:34AM +0200, Matthias Bolte wrote:
2010/7/26 Daniel P. Berrange <berrange@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 :|

2010/7/27 Daniel P. Berrange <berrange@redhat.com>:
On Tue, Jul 27, 2010 at 11:46:34AM +0200, Matthias Bolte wrote:
2010/7/26 Daniel P. Berrange <berrange@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

On Tue, Jul 27, 2010 at 12:49:45PM +0200, Matthias Bolte wrote:
2010/7/27 Daniel P. Berrange <berrange@redhat.com>:
On Tue, Jul 27, 2010 at 11:46:34AM +0200, Matthias Bolte wrote:
2010/7/26 Daniel P. Berrange <berrange@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 :|

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

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

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

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

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

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

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

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

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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

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@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

2010/7/24 Daniel Veillard <veillard@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
participants (5)
-
Daniel P. Berrange
-
Daniel Veillard
-
Eric Blake
-
Jamie Strandboge
-
Matthias Bolte