Format probing will be dropped so remove the tests which will become obsolete. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../qemuxml2argvdata/disk-many-format-probing.args | 32 ------------- .../qemuxml2argvdata/disk-many-format-probing.xml | 48 -------------------- tests/qemuxml2argvtest.c | 6 --- .../disk-many-format-probing.xml | 52 ---------------------- tests/qemuxml2xmltest.c | 6 --- 5 files changed, 144 deletions(-) delete mode 100644 tests/qemuxml2argvdata/disk-many-format-probing.args delete mode 100644 tests/qemuxml2argvdata/disk-many-format-probing.xml delete mode 100644 tests/qemuxml2xmloutdata/disk-many-format-probing.xml diff --git a/tests/qemuxml2argvdata/disk-many-format-probing.args b/tests/qemuxml2argvdata/disk-many-format-probing.args deleted file mode 100644 index 8980b80f92..0000000000 --- a/tests/qemuxml2argvdata/disk-many-format-probing.args +++ /dev/null @@ -1,32 +0,0 @@ -LC_ALL=C \ -PATH=/bin \ -HOME=/home/test \ -USER=test \ -LOGNAME=test \ -QEMU_AUDIO_DRV=none \ -/usr/bin/qemu-system-i686 \ --name QEMUGuest1 \ --S \ --machine pc,accel=tcg,usb=off,dump-guest-core=off \ --m 214 \ --smp 1,sockets=1,cores=1,threads=1 \ --uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ --display none \ --no-user-config \ --nodefaults \ --chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ -server,nowait \ --mon chardev=charmonitor,id=monitor,mode=control \ --rtc base=utc \ --no-shutdown \ --no-acpi \ --boot c \ --usb \ --drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-ide0-0-0 \ --device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ --drive file=/dev/HostVG/QEMUGuest2,if=none,id=drive-ide0-0-1 \ --device ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \ --drive file=/tmp/data.img,if=none,id=drive-ide0-1-0 \ --device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \ --drive file=/tmp/logs.img,if=none,id=drive-ide0-1-1 \ --device ide-drive,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 diff --git a/tests/qemuxml2argvdata/disk-many-format-probing.xml b/tests/qemuxml2argvdata/disk-many-format-probing.xml deleted file mode 100644 index 5e34eb46e0..0000000000 --- a/tests/qemuxml2argvdata/disk-many-format-probing.xml +++ /dev/null @@ -1,48 +0,0 @@ -<domain type='qemu'> - <name>QEMUGuest1</name> - <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219136</currentMemory> - <vcpu placement='static'>1</vcpu> - <os> - <type arch='i686' machine='pc'>hvm</type> - <boot dev='hd'/> - </os> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu-system-i686</emulator> - <disk type='block' device='disk'> - <driver name='qemu'/> - <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='hda' bus='ide'/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> - </disk> - <disk type='block' device='disk'> - <driver name='qemu'/> - <source dev='/dev/HostVG/QEMUGuest2'/> - <target dev='hdb' bus='ide'/> - <address type='drive' controller='0' bus='0' target='0' unit='1'/> - </disk> - <disk type='file' device='disk'> - <driver name='qemu'/> - <source file='/tmp/data.img'/> - <target dev='hdc' bus='ide'/> - <address type='drive' controller='0' bus='1' target='0' unit='0'/> - </disk> - <disk type='file' device='disk'> - <driver name='qemu'/> - <source file='/tmp/logs.img'/> - <target dev='hdd' bus='ide'/> - <address type='drive' controller='0' bus='1' target='0' unit='1'/> - </disk> - <controller type='usb' index='0'/> - <controller type='ide' index='0'/> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <memballoon model='none'/> - </devices> -</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 14a994523f..0f859ca3bc 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2848,12 +2848,6 @@ mymain(void) DO_TEST_CAPS_LATEST("disk-virtio-scsi-reservations"); - /* Test disks with format probing enabled for legacy reasons. - * New tests should not go in this section. */ - driver.config->allowDiskFormatProbing = true; - DO_TEST("disk-many-format-probing", QEMU_CAPS_DRIVE_BOOT); - driver.config->allowDiskFormatProbing = false; - DO_TEST("video-virtio-gpu-ccw", QEMU_CAPS_CCW, QEMU_CAPS_DEVICE_VIRTIO_GPU, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, diff --git a/tests/qemuxml2xmloutdata/disk-many-format-probing.xml b/tests/qemuxml2xmloutdata/disk-many-format-probing.xml deleted file mode 100644 index 914b9b0937..0000000000 --- a/tests/qemuxml2xmloutdata/disk-many-format-probing.xml +++ /dev/null @@ -1,52 +0,0 @@ -<domain type='qemu'> - <name>QEMUGuest1</name> - <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219136</currentMemory> - <vcpu placement='static'>1</vcpu> - <os> - <type arch='i686' machine='pc'>hvm</type> - <boot dev='hd'/> - </os> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu-system-i686</emulator> - <disk type='block' device='disk'> - <driver name='qemu'/> - <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='hda' bus='ide'/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> - </disk> - <disk type='block' device='disk'> - <driver name='qemu'/> - <source dev='/dev/HostVG/QEMUGuest2'/> - <target dev='hdb' bus='ide'/> - <address type='drive' controller='0' bus='0' target='0' unit='1'/> - </disk> - <disk type='file' device='disk'> - <driver name='qemu'/> - <source file='/tmp/data.img'/> - <target dev='hdc' bus='ide'/> - <address type='drive' controller='0' bus='1' target='0' unit='0'/> - </disk> - <disk type='file' device='disk'> - <driver name='qemu'/> - <source file='/tmp/logs.img'/> - <target dev='hdd' bus='ide'/> - <address type='drive' controller='0' bus='1' target='0' unit='1'/> - </disk> - <controller type='usb' index='0'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> - </controller> - <controller type='ide' index='0'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> - </controller> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <memballoon model='none'/> - </devices> -</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index f53f9a7db5..a6be502cf7 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1183,12 +1183,6 @@ mymain(void) QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW, QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW); - /* Test disks with format probing enabled for legacy reasons. - * New tests should not go in this section. */ - driver.config->allowDiskFormatProbing = true; - DO_TEST("disk-many-format-probing", NONE); - driver.config->allowDiskFormatProbing = false; - # define DO_TEST_STATUS(name) \ do { \ if (testInfoSetStatus(&info, name, GIC_NONE) < 0) { \ -- 2.16.2

The option is insecure and it has been long enough for users to migrate their disk files to use explicit format. Drop the option and related code. The config parser still parses it and rejects statup if it's still present in the config in enabled state. The augeas lens is also kept so that users can disable it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu.conf | 12 --------- src/qemu/qemu_conf.c | 8 +++++- src/qemu/qemu_conf.h | 1 - src/qemu/qemu_domain.c | 54 +++++++++++--------------------------- src/qemu/qemu_driver.c | 20 +++++--------- src/qemu/test_libvirtd_qemu.aug.in | 1 - 6 files changed, 28 insertions(+), 68 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index c8e1a62d1c..9be4d2c1e7 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -650,18 +650,6 @@ #relaxed_acs_check = 1 -# If allow_disk_format_probing is enabled, libvirt will probe disk -# images to attempt to identify their format, when not otherwise -# specified in the XML. This is disabled by default. -# -# WARNING: Enabling probing is a security hole in almost all -# deployments. It is strongly recommended that users update their -# guest XML <disk> elements to include <driver type='XXXX'/> -# elements instead of enabling this option. -# -#allow_disk_format_probing = 1 - - # In order to prevent accidentally starting two domains that # share one writable disk, libvirt offers two approaches for # locking files. The first one is sanlock, the other one, diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 5f35a49e91..873c5fd123 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -481,6 +481,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, char **nvram = NULL; char *corestr = NULL; char **namespaces = NULL; + bool tmp; /* Just check the file is readable before opening it, otherwise * libvirt emits an error. @@ -773,8 +774,13 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, goto cleanup; if (virConfGetValueBool(conf, "clear_emulator_capabilities", &cfg->clearEmulatorCapabilities) < 0) goto cleanup; - if (virConfGetValueBool(conf, "allow_disk_format_probing", &cfg->allowDiskFormatProbing) < 0) + if (virConfGetValueBool(conf, "allow_disk_format_probing", &tmp) < 0) goto cleanup; + if (tmp) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("allow_disk_format_probing is no longer supported")); + goto cleanup; + } if (virConfGetValueBool(conf, "set_process_name", &cfg->setProcessName) < 0) goto cleanup; if (virConfGetValueUInt(conf, "max_processes", &cfg->maxProcesses) < 0) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 6d25c3e74f..be7d5a1e96 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -161,7 +161,6 @@ struct _virQEMUDriverConfig { bool vncAllowHostAudio; bool nogfxAllowHostAudio; bool clearEmulatorCapabilities; - bool allowDiskFormatProbing; bool setProcessName; unsigned int maxProcesses; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a7c76c1afd..d230ead359 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5917,39 +5917,23 @@ qemuDomainDeviceDiskDefPostParseRestoreSecAlias(virDomainDiskDefPtr disk, static int qemuDomainDeviceDiskDefPostParse(virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps, - unsigned int parseFlags, - virQEMUDriverConfigPtr cfg) + unsigned int parseFlags) { /* set default disk types and drivers */ - /* assign default storage format and driver according to config */ - if (cfg->allowDiskFormatProbing) { - /* default disk format for drives */ - if (virDomainDiskGetFormat(disk) == VIR_STORAGE_FILE_NONE && - (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_FILE || - virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_BLOCK)) - virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_AUTO); - - /* default disk format for mirrored drive */ - if (disk->mirror && - disk->mirror->format == VIR_STORAGE_FILE_NONE) - disk->mirror->format = VIR_STORAGE_FILE_AUTO; - } else { - /* default driver if probing is forbidden */ - if (!virDomainDiskGetDriver(disk) && - virDomainDiskSetDriver(disk, "qemu") < 0) - return -1; + if (!virDomainDiskGetDriver(disk) && + virDomainDiskSetDriver(disk, "qemu") < 0) + return -1; - /* default disk format for drives */ - if (virDomainDiskGetFormat(disk) == VIR_STORAGE_FILE_NONE && - (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_FILE || - virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_BLOCK)) - virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW); + /* default disk format for drives */ + if (virDomainDiskGetFormat(disk) == VIR_STORAGE_FILE_NONE && + (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_FILE || + virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_BLOCK)) + virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW); - /* default disk format for mirrored drive */ - if (disk->mirror && - disk->mirror->format == VIR_STORAGE_FILE_NONE) - disk->mirror->format = VIR_STORAGE_FILE_RAW; - } + /* default disk format for mirrored drive */ + if (disk->mirror && + disk->mirror->format == VIR_STORAGE_FILE_NONE) + disk->mirror->format = VIR_STORAGE_FILE_RAW; if (qemuDomainDeviceDiskDefPostParseRestoreSecAlias(disk, qemuCaps, parseFlags) < 0) @@ -6044,7 +6028,6 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, * function shall not fail in that case. It will be re-run on VM startup * with the capabilities populated. */ virQEMUCapsPtr qemuCaps = parseOpaque; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); int ret = -1; switch ((virDomainDeviceType) dev->type) { @@ -6054,7 +6037,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, case VIR_DOMAIN_DEVICE_DISK: ret = qemuDomainDeviceDiskDefPostParse(dev->data.disk, qemuCaps, - parseFlags, cfg); + parseFlags); break; case VIR_DOMAIN_DEVICE_VIDEO: @@ -6112,7 +6095,6 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, break; } - virObjectUnref(cfg); return ret; } @@ -7126,11 +7108,6 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver, qemuDomainLogContextPtr logCtxt) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - int format = virDomainDiskGetFormat(disk); - - if ((!format || format == VIR_STORAGE_FILE_AUTO) && - cfg->allowDiskFormatProbing) - qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_DISK_PROBING, logCtxt); if (disk->rawio == VIR_TRISTATE_BOOL_YES) qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, @@ -8085,8 +8062,7 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, qemuDomainGetImageIds(cfg, vm, src, disk->src, &uid, &gid); if (virStorageFileGetMetadata(src, - uid, gid, - cfg->allowDiskFormatProbing, + uid, gid, false, report_broken) < 0) goto cleanup; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b7c3348550..0283a906af 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -365,8 +365,6 @@ qemuSecurityInit(virQEMUDriverPtr driver) virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); unsigned int flags = 0; - if (cfg->allowDiskFormatProbing) - flags |= VIR_SECURITY_MANAGER_ALLOW_DISK_PROBE; if (cfg->securityDefaultConfined) flags |= VIR_SECURITY_MANAGER_DEFAULT_CONFINED; if (cfg->securityRequireConfined) @@ -11939,8 +11937,7 @@ qemuStorageLimitsRefresh(virQEMUDriverPtr driver, if (virStorageSourceUpdateBackingSizes(src, fd, &sb) < 0) goto cleanup; - if (virStorageSourceUpdateCapacity(src, buf, len, - cfg->allowDiskFormatProbing) < 0) + if (virStorageSourceUpdateCapacity(src, buf, len, false) < 0) goto cleanup; /* If guest is not using raw disk format and is on a host block @@ -14169,16 +14166,11 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, defdisk->src->path, virStorageFileFormatTypeToString(defdisk->src->format)); } else { - if (!cfg->allowDiskFormatProbing) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown image format of '%s' and " - "format probing is disabled"), - defdisk->src->path); - goto cleanup; - } - - /* adds cmd line arg: backing_file=/path/to/backing/file */ - virCommandAddArgFormat(cmd, "backing_file=%s", defdisk->src->path); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown image format of '%s' and " + "format probing is disabled"), + defdisk->src->path); + goto cleanup; } /* adds cmd line args: /path/to/target/file */ diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 912161c272..587e8f6521 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -79,7 +79,6 @@ module Test_libvirtd_qemu = { "dump_guest_core" = "1" } { "mac_filter" = "1" } { "relaxed_acs_check" = "1" } -{ "allow_disk_format_probing" = "1" } { "lock_manager" = "lockd" } { "max_queued" = "0" } { "keepalive_interval" = "5" } -- 2.16.2

Previous patch naively removed all code relevant to disk format checking. The semantics now dictate that the format check when creating external snapshots is now impossible as we always fill in the format for disks in domain definition in the post-parse callback. Remove the impossible code path. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0283a906af..156f5d52ce 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14160,18 +14160,10 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, NULL))) goto cleanup; - if (defdisk->src->format > 0) { - /* adds cmd line arg: backing_file=/path/to/backing/file,backing_fmd=format */ - virCommandAddArgFormat(cmd, "backing_file=%s,backing_fmt=%s", - defdisk->src->path, - virStorageFileFormatTypeToString(defdisk->src->format)); - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown image format of '%s' and " - "format probing is disabled"), - defdisk->src->path); - goto cleanup; - } + /* adds cmd line arg: backing_file=/path/to/backing/file,backing_fmd=format */ + virCommandAddArgFormat(cmd, "backing_file=%s,backing_fmt=%s", + defdisk->src->path, + virStorageFileFormatTypeToString(defdisk->src->format)); /* adds cmd line args: /path/to/target/file */ virCommandAddArg(cmd, snapdisk->src->path); -- 2.16.2

Nothing is setting that flag now so it can be removed. Note that removing 'mgr' from 'load_profile' in the apparmor driver would create a lot of churn. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/security/security_apparmor.c | 6 ++---- src/security/security_manager.c | 7 ------- src/security/security_manager.h | 5 +---- 3 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 333d098be4..cb41df71a9 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -170,7 +170,7 @@ profile_status_file(const char *str) * load (add) a profile. Will create one if necessary */ static int -load_profile(virSecurityManagerPtr mgr, +load_profile(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, const char *profile, virDomainDefPtr def, const char *fn, @@ -180,8 +180,6 @@ load_profile(virSecurityManagerPtr mgr, bool create = true; char *xml = NULL; virCommandPtr cmd = NULL; - const char *probe = virSecurityManagerGetAllowDiskFormatProbing(mgr) - ? "1" : "0"; xml = virDomainDefFormat(def, NULL, VIR_DOMAIN_DEF_FORMAT_SECURE); if (!xml) @@ -190,7 +188,7 @@ load_profile(virSecurityManagerPtr mgr, if (profile_status_file(profile) >= 0) create = false; - cmd = virCommandNewArgList(VIRT_AA_HELPER, "-p", probe, + cmd = virCommandNewArgList(VIRT_AA_HELPER, create ? "-c" : "-r", "-u", profile, NULL); if (!create && fn) { diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 71f7f59b9c..cacd1d5457 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -365,13 +365,6 @@ virSecurityManagerGetBaseLabel(virSecurityManagerPtr mgr, } -bool -virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr) -{ - return mgr->flags & VIR_SECURITY_MANAGER_ALLOW_DISK_PROBE; -} - - bool virSecurityManagerGetDefaultConfined(virSecurityManagerPtr mgr) { diff --git a/src/security/security_manager.h b/src/security/security_manager.h index c36a8b488f..c5e472bba4 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -31,7 +31,6 @@ typedef struct _virSecurityManager virSecurityManager; typedef virSecurityManager *virSecurityManagerPtr; typedef enum { - VIR_SECURITY_MANAGER_ALLOW_DISK_PROBE = 1 << 0, VIR_SECURITY_MANAGER_DEFAULT_CONFINED = 1 << 1, VIR_SECURITY_MANAGER_REQUIRE_CONFINED = 1 << 2, VIR_SECURITY_MANAGER_PRIVILEGED = 1 << 3, @@ -40,8 +39,7 @@ typedef enum { } virSecurityManagerNewFlags; # define VIR_SECURITY_MANAGER_NEW_MASK \ - (VIR_SECURITY_MANAGER_ALLOW_DISK_PROBE | \ - VIR_SECURITY_MANAGER_DEFAULT_CONFINED | \ + (VIR_SECURITY_MANAGER_DEFAULT_CONFINED | \ VIR_SECURITY_MANAGER_REQUIRE_CONFINED | \ VIR_SECURITY_MANAGER_PRIVILEGED) @@ -89,7 +87,6 @@ const char *virSecurityManagerGetDOI(virSecurityManagerPtr mgr); const char *virSecurityManagerGetModel(virSecurityManagerPtr mgr); const char *virSecurityManagerGetBaseLabel(virSecurityManagerPtr mgr, int virtType); -bool virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr); bool virSecurityManagerGetDefaultConfined(virSecurityManagerPtr mgr); bool virSecurityManagerGetRequireConfined(virSecurityManagerPtr mgr); bool virSecurityManagerGetPrivileged(virSecurityManagerPtr mgr); -- 2.16.2

As the aa-helper binary is supposed to be used only with libvirt, we can fully remove it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/security/virt-aa-helper.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index d0f9876da5..37fe36a329 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -63,7 +63,6 @@ static char *progname; typedef struct { - bool allowDiskFormatProbing; char uuid[PROFILE_NAME_SIZE]; /* UUID of vm */ bool dryrun; /* dry run */ char cmd; /* 'c' create @@ -115,7 +114,6 @@ vah_usage(void) " -r | --replace reload profile\n" " -R | --remove unload profile\n" " -h | --help this help\n" - " -p | --probing [0|1] allow disk format probing\n" " -u | --uuid <uuid> uuid (profile name)\n" "\n"), progname); @@ -975,10 +973,8 @@ get_files(vahControl * ctl) /* XXX - if we knew the qemu user:group here we could send it in * so that the open could be re-tried as that user:group. */ - if (!virStorageSourceHasBacking(disk->src)) { - bool probe = ctl->allowDiskFormatProbing; - virStorageFileGetMetadata(disk->src, -1, -1, probe, false); - } + if (!virStorageSourceHasBacking(disk->src)) + virStorageFileGetMetadata(disk->src, -1, -1, false, false); /* XXX passing ignoreOpenFailure = true to get back to the behavior * from before using virDomainDiskDefForeachPath. actually we should @@ -1228,7 +1224,6 @@ vahParseArgv(vahControl * ctl, int argc, char **argv) { int arg, idx = 0; struct option opt[] = { - {"probing", 1, 0, 'p' }, {"add", 0, 0, 'a'}, {"create", 0, 0, 'c'}, {"dryrun", 0, 0, 'd'}, @@ -1280,12 +1275,6 @@ vahParseArgv(vahControl * ctl, int argc, char **argv) PROFILE_NAME_SIZE) == NULL) vah_error(ctl, 1, _("error copying UUID")); break; - case 'p': - if (STREQ(optarg, "1")) - ctl->allowDiskFormatProbing = true; - else - ctl->allowDiskFormatProbing = false; - break; default: vah_error(ctl, 1, _("unsupported option")); break; -- 2.16.2

We have a test case for QED disk image with autodetection but not with the format explicitly specified. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virstoragetest.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 0e11602e84..f3b96a9b87 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -949,6 +949,9 @@ mymain(void) .type = VIR_STORAGE_TYPE_FILE, .format = VIR_STORAGE_FILE_RAW, }; + TEST_CHAIN(absqed, VIR_STORAGE_FILE_QED, + (&qed, &raw), EXP_PASS, + (&qed, &raw), ALLOW_PROBE | EXP_PASS); TEST_CHAIN(absqed, VIR_STORAGE_FILE_AUTO, (&qed_as_raw), EXP_PASS, (&qed, &raw), ALLOW_PROBE | EXP_PASS); -- 2.16.2

The second set of arguments for TEST_CHAIN always specifies the 'ALLOW_PROBE' flag. Make it part of the macro. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virstoragetest.c | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index f3b96a9b87..0fe8ac7e7f 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -757,7 +757,7 @@ mymain(void) #define TEST_CHAIN(path, format, chain1, flags1, chain2, flags2) \ do { \ TEST_ONE_CHAIN(path, format, flags1, VIR_FLATTEN_1(chain1)); \ - TEST_ONE_CHAIN(path, format, flags2, VIR_FLATTEN_1(chain2)); \ + TEST_ONE_CHAIN(path, format, ALLOW_PROBE | flags2, VIR_FLATTEN_1(chain2)); \ } while (0) /* The actual tests, in several groups. */ @@ -774,10 +774,10 @@ mymain(void) }; TEST_CHAIN(absraw, VIR_STORAGE_FILE_RAW, (&raw), EXP_PASS, - (&raw), ALLOW_PROBE | EXP_PASS); + (&raw), EXP_PASS); TEST_CHAIN(absraw, VIR_STORAGE_FILE_AUTO, (&raw), EXP_PASS, - (&raw), ALLOW_PROBE | EXP_PASS); + (&raw), EXP_PASS); /* Qcow2 file with relative raw backing, format provided */ raw.pathRel = "raw"; @@ -795,10 +795,10 @@ mymain(void) }; TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &raw), EXP_PASS, - (&qcow2, &raw), ALLOW_PROBE | EXP_PASS); + (&qcow2, &raw), EXP_PASS); TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_AUTO, (&qcow2_as_raw), EXP_PASS, - (&qcow2, &raw), ALLOW_PROBE | EXP_PASS); + (&qcow2, &raw), EXP_PASS); /* Rewrite qcow2 file to use absolute backing name */ virCommandFree(cmd); @@ -812,10 +812,10 @@ mymain(void) /* Qcow2 file with raw as absolute backing, backing format provided */ TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &raw), EXP_PASS, - (&qcow2, &raw), ALLOW_PROBE | EXP_PASS); + (&qcow2, &raw), EXP_PASS); TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_AUTO, (&qcow2_as_raw), EXP_PASS, - (&qcow2, &raw), ALLOW_PROBE | EXP_PASS); + (&qcow2, &raw), EXP_PASS); /* Wrapped file access */ testFileData wrap = { @@ -827,7 +827,7 @@ mymain(void) }; TEST_CHAIN(abswrap, VIR_STORAGE_FILE_QCOW2, (&wrap, &qcow2, &raw), EXP_PASS, - (&wrap, &qcow2, &raw), ALLOW_PROBE | EXP_PASS); + (&wrap, &qcow2, &raw), EXP_PASS); /* Rewrite qcow2 and wrap file to omit backing file type */ virCommandFree(cmd); @@ -852,7 +852,7 @@ mymain(void) }; TEST_CHAIN(abswrap, VIR_STORAGE_FILE_QCOW2, (&wrap_as_raw, &qcow2_as_raw), EXP_PASS, - (&wrap, &qcow2, &raw), ALLOW_PROBE | EXP_PASS); + (&wrap, &qcow2, &raw), EXP_PASS); /* Rewrite qcow2 to a missing backing file, with backing type */ virCommandFree(cmd); @@ -866,7 +866,7 @@ mymain(void) /* Qcow2 file with missing backing file but specified type */ TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2), EXP_WARN, - (&qcow2), ALLOW_PROBE | EXP_WARN); + (&qcow2), EXP_WARN); /* Rewrite qcow2 to a missing backing file, without backing type */ virCommandFree(cmd); @@ -878,7 +878,7 @@ mymain(void) /* Qcow2 file with missing backing file and no specified type */ TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2), EXP_WARN, - (&qcow2), ALLOW_PROBE | EXP_WARN); + (&qcow2), EXP_WARN); /* Rewrite qcow2 to use an nbd: protocol as backend */ virCommandFree(cmd); @@ -899,7 +899,7 @@ mymain(void) }; TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &nbd), EXP_PASS, - (&qcow2, &nbd), ALLOW_PROBE | EXP_PASS); + (&qcow2, &nbd), EXP_PASS); /* Rewrite qcow2 to use an nbd: protocol as backend */ virCommandFree(cmd); @@ -920,7 +920,7 @@ mymain(void) }; TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &nbd2), EXP_PASS, - (&qcow2, &nbd2), ALLOW_PROBE | EXP_PASS); + (&qcow2, &nbd2), EXP_PASS); /* Rewrite qcow2 to use an nbd: protocol without path as backend */ virCommandFree(cmd); @@ -934,7 +934,7 @@ mymain(void) nbd2.path = NULL; TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &nbd2), EXP_PASS, - (&qcow2, &nbd2), ALLOW_PROBE | EXP_PASS); + (&qcow2, &nbd2), EXP_PASS); /* qed file */ testFileData qed = { @@ -951,10 +951,10 @@ mymain(void) }; TEST_CHAIN(absqed, VIR_STORAGE_FILE_QED, (&qed, &raw), EXP_PASS, - (&qed, &raw), ALLOW_PROBE | EXP_PASS); + (&qed, &raw), EXP_PASS); TEST_CHAIN(absqed, VIR_STORAGE_FILE_AUTO, (&qed_as_raw), EXP_PASS, - (&qed, &raw), ALLOW_PROBE | EXP_PASS); + (&qed, &raw), EXP_PASS); /* directory */ testFileData dir = { @@ -969,13 +969,13 @@ mymain(void) }; TEST_CHAIN(absdir, VIR_STORAGE_FILE_RAW, (&dir_as_raw), EXP_PASS, - (&dir_as_raw), ALLOW_PROBE | EXP_PASS); + (&dir_as_raw), EXP_PASS); TEST_CHAIN(absdir, VIR_STORAGE_FILE_NONE, (&dir), EXP_PASS, - (&dir), ALLOW_PROBE | EXP_PASS); + (&dir), EXP_PASS); TEST_CHAIN(absdir, VIR_STORAGE_FILE_DIR, (&dir), EXP_PASS, - (&dir), ALLOW_PROBE | EXP_PASS); + (&dir), EXP_PASS); #ifdef HAVE_SYMLINK /* Rewrite qcow2 and wrap file to use backing names relative to a @@ -1014,7 +1014,7 @@ mymain(void) raw.pathRel = "../raw"; TEST_CHAIN(abslink2, VIR_STORAGE_FILE_QCOW2, (&link2, &link1, &raw), EXP_PASS, - (&link2, &link1, &raw), ALLOW_PROBE | EXP_PASS); + (&link2, &link1, &raw), EXP_PASS); #endif /* Rewrite qcow2 to be a self-referential loop */ @@ -1028,7 +1028,7 @@ mymain(void) /* Behavior of an infinite loop chain */ TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2), EXP_WARN, - (&qcow2), ALLOW_PROBE | EXP_WARN); + (&qcow2), EXP_WARN); /* Rewrite wrap and qcow2 to be mutually-referential loop */ virCommandFree(cmd); @@ -1047,7 +1047,7 @@ mymain(void) /* Behavior of an infinite loop chain */ TEST_CHAIN(abswrap, VIR_STORAGE_FILE_QCOW2, (&wrap, &qcow2), EXP_WARN, - (&wrap, &qcow2), ALLOW_PROBE | EXP_WARN); + (&wrap, &qcow2), EXP_WARN); /* Rewrite qcow2 to use an rbd: protocol as backend */ virCommandFree(cmd); @@ -1067,7 +1067,7 @@ mymain(void) }; TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &rbd1), EXP_PASS, - (&qcow2, &rbd1), ALLOW_PROBE | EXP_PASS); + (&qcow2, &rbd1), EXP_PASS); /* Rewrite qcow2 to use an rbd: protocol as backend */ virCommandFree(cmd); @@ -1089,7 +1089,7 @@ mymain(void) }; TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &rbd2), EXP_PASS, - (&qcow2, &rbd2), ALLOW_PROBE | EXP_PASS); + (&qcow2, &rbd2), EXP_PASS); /* Rewrite wrap and qcow2 back to 3-deep chain, absolute backing */ -- 2.16.2

Storage drivers now don't allow it so there's no need to test it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virstoragetest.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 0fe8ac7e7f..4d98310ee1 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -95,8 +95,7 @@ testCleanupImages(void) static virStorageSourcePtr testStorageFileGetMetadata(const char *path, int format, - uid_t uid, gid_t gid, - bool allow_probe) + uid_t uid, gid_t gid) { struct stat st; virStorageSourcePtr ret = NULL; @@ -118,7 +117,7 @@ testStorageFileGetMetadata(const char *path, if (VIR_STRDUP(ret->path, path) < 0) goto error; - if (virStorageFileGetMetadata(ret, uid, gid, allow_probe, false) < 0) + if (virStorageFileGetMetadata(ret, uid, gid, false, false) < 0) goto error; return ret; @@ -320,8 +319,7 @@ testStorageChain(const void *args) size_t i = 0; char *broken = NULL; - meta = testStorageFileGetMetadata(data->start, data->format, -1, -1, - (data->flags & ALLOW_PROBE) != 0); + meta = testStorageFileGetMetadata(data->start, data->format, -1, -1); if (!meta) { if (data->flags & EXP_FAIL) { virResetLastError(); @@ -757,7 +755,7 @@ mymain(void) #define TEST_CHAIN(path, format, chain1, flags1, chain2, flags2) \ do { \ TEST_ONE_CHAIN(path, format, flags1, VIR_FLATTEN_1(chain1)); \ - TEST_ONE_CHAIN(path, format, ALLOW_PROBE | flags2, VIR_FLATTEN_1(chain2)); \ + TEST_ONE_CHAIN(path, format, flags2, VIR_FLATTEN_1(chain2)); \ } while (0) /* The actual tests, in several groups. */ @@ -798,7 +796,7 @@ mymain(void) (&qcow2, &raw), EXP_PASS); TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_AUTO, (&qcow2_as_raw), EXP_PASS, - (&qcow2, &raw), EXP_PASS); + (&qcow2_as_raw), EXP_PASS); /* Rewrite qcow2 file to use absolute backing name */ virCommandFree(cmd); @@ -815,7 +813,7 @@ mymain(void) (&qcow2, &raw), EXP_PASS); TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_AUTO, (&qcow2_as_raw), EXP_PASS, - (&qcow2, &raw), EXP_PASS); + (&qcow2_as_raw), EXP_PASS); /* Wrapped file access */ testFileData wrap = { @@ -852,7 +850,7 @@ mymain(void) }; TEST_CHAIN(abswrap, VIR_STORAGE_FILE_QCOW2, (&wrap_as_raw, &qcow2_as_raw), EXP_PASS, - (&wrap, &qcow2, &raw), EXP_PASS); + (&wrap_as_raw, &qcow2_as_raw), EXP_PASS); /* Rewrite qcow2 to a missing backing file, with backing type */ virCommandFree(cmd); @@ -954,7 +952,7 @@ mymain(void) (&qed, &raw), EXP_PASS); TEST_CHAIN(absqed, VIR_STORAGE_FILE_AUTO, (&qed_as_raw), EXP_PASS, - (&qed, &raw), EXP_PASS); + (&qed_as_raw), EXP_PASS); /* directory */ testFileData dir = { @@ -1101,7 +1099,7 @@ mymain(void) /* Test behavior of chain lookups, absolute backing from relative start */ chain = testStorageFileGetMetadata("wrap", VIR_STORAGE_FILE_QCOW2, - -1, -1, false); + -1, -1); if (!chain) { ret = -1; goto cleanup; @@ -1166,8 +1164,7 @@ mymain(void) /* Test behavior of chain lookups, relative backing from absolute start */ virStorageSourceFree(chain); - chain = testStorageFileGetMetadata(abswrap, VIR_STORAGE_FILE_QCOW2, - -1, -1, false); + chain = testStorageFileGetMetadata(abswrap, VIR_STORAGE_FILE_QCOW2, -1, -1); if (!chain) { ret = -1; goto cleanup; @@ -1214,7 +1211,7 @@ mymain(void) /* Test behavior of chain lookups, relative backing */ virStorageSourceFree(chain); chain = testStorageFileGetMetadata("sub/link2", VIR_STORAGE_FILE_QCOW2, - -1, -1, false); + -1, -1); if (!chain) { ret = -1; goto cleanup; -- 2.16.2

Now that all test cases with TEST_CHAIN were testing the same thing twice drop one of them. Note that some of the cases were duplicate even before dropping the image format probing tests. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virstoragetest.c | 93 +++++++++++++------------------------------------- 1 file changed, 23 insertions(+), 70 deletions(-) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 4d98310ee1..2236351309 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -752,11 +752,8 @@ mymain(void) #define VIR_FLATTEN_2(...) __VA_ARGS__ #define VIR_FLATTEN_1(_1) VIR_FLATTEN_2 _1 -#define TEST_CHAIN(path, format, chain1, flags1, chain2, flags2) \ - do { \ - TEST_ONE_CHAIN(path, format, flags1, VIR_FLATTEN_1(chain1)); \ - TEST_ONE_CHAIN(path, format, flags2, VIR_FLATTEN_1(chain2)); \ - } while (0) +#define TEST_CHAIN(path, format, chain, flags) \ + TEST_ONE_CHAIN(path, format, flags, VIR_FLATTEN_1(chain)); /* The actual tests, in several groups. */ virTestCounterReset("Storage backing chain "); @@ -770,12 +767,8 @@ mymain(void) .type = VIR_STORAGE_TYPE_FILE, .format = VIR_STORAGE_FILE_RAW, }; - TEST_CHAIN(absraw, VIR_STORAGE_FILE_RAW, - (&raw), EXP_PASS, - (&raw), EXP_PASS); - TEST_CHAIN(absraw, VIR_STORAGE_FILE_AUTO, - (&raw), EXP_PASS, - (&raw), EXP_PASS); + TEST_CHAIN(absraw, VIR_STORAGE_FILE_RAW, (&raw), EXP_PASS); + TEST_CHAIN(absraw, VIR_STORAGE_FILE_AUTO, (&raw), EXP_PASS); /* Qcow2 file with relative raw backing, format provided */ raw.pathRel = "raw"; @@ -791,12 +784,8 @@ mymain(void) .type = VIR_STORAGE_TYPE_FILE, .format = VIR_STORAGE_FILE_RAW, }; - TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, - (&qcow2, &raw), EXP_PASS, - (&qcow2, &raw), EXP_PASS); - TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_AUTO, - (&qcow2_as_raw), EXP_PASS, - (&qcow2_as_raw), EXP_PASS); + TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &raw), EXP_PASS); + TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_AUTO, (&qcow2_as_raw), EXP_PASS); /* Rewrite qcow2 file to use absolute backing name */ virCommandFree(cmd); @@ -808,12 +797,8 @@ mymain(void) raw.pathRel = NULL; /* Qcow2 file with raw as absolute backing, backing format provided */ - TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, - (&qcow2, &raw), EXP_PASS, - (&qcow2, &raw), EXP_PASS); - TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_AUTO, - (&qcow2_as_raw), EXP_PASS, - (&qcow2_as_raw), EXP_PASS); + TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &raw), EXP_PASS); + TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_AUTO, (&qcow2_as_raw), EXP_PASS); /* Wrapped file access */ testFileData wrap = { @@ -823,9 +808,7 @@ mymain(void) .type = VIR_STORAGE_TYPE_FILE, .format = VIR_STORAGE_FILE_QCOW2, }; - TEST_CHAIN(abswrap, VIR_STORAGE_FILE_QCOW2, - (&wrap, &qcow2, &raw), EXP_PASS, - (&wrap, &qcow2, &raw), EXP_PASS); + TEST_CHAIN(abswrap, VIR_STORAGE_FILE_QCOW2, (&wrap, &qcow2, &raw), EXP_PASS); /* Rewrite qcow2 and wrap file to omit backing file type */ virCommandFree(cmd); @@ -849,7 +832,6 @@ mymain(void) .format = VIR_STORAGE_FILE_QCOW2, }; TEST_CHAIN(abswrap, VIR_STORAGE_FILE_QCOW2, - (&wrap_as_raw, &qcow2_as_raw), EXP_PASS, (&wrap_as_raw, &qcow2_as_raw), EXP_PASS); /* Rewrite qcow2 to a missing backing file, with backing type */ @@ -862,9 +844,7 @@ mymain(void) qcow2.expBackingStoreRaw = datadir "/bogus"; /* Qcow2 file with missing backing file but specified type */ - TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, - (&qcow2), EXP_WARN, - (&qcow2), EXP_WARN); + TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2), EXP_WARN); /* Rewrite qcow2 to a missing backing file, without backing type */ virCommandFree(cmd); @@ -874,9 +854,7 @@ mymain(void) ret = -1; /* Qcow2 file with missing backing file and no specified type */ - TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, - (&qcow2), EXP_WARN, - (&qcow2), EXP_WARN); + TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2), EXP_WARN); /* Rewrite qcow2 to use an nbd: protocol as backend */ virCommandFree(cmd); @@ -895,9 +873,7 @@ mymain(void) .protocol = VIR_STORAGE_NET_PROTOCOL_NBD, .hostname = "example.org", }; - TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, - (&qcow2, &nbd), EXP_PASS, - (&qcow2, &nbd), EXP_PASS); + TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &nbd), EXP_PASS); /* Rewrite qcow2 to use an nbd: protocol as backend */ virCommandFree(cmd); @@ -916,9 +892,7 @@ mymain(void) .protocol = VIR_STORAGE_NET_PROTOCOL_NBD, .hostname = "example.org", }; - TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, - (&qcow2, &nbd2), EXP_PASS, - (&qcow2, &nbd2), EXP_PASS); + TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &nbd2), EXP_PASS); /* Rewrite qcow2 to use an nbd: protocol without path as backend */ virCommandFree(cmd); @@ -930,9 +904,7 @@ mymain(void) qcow2.expBackingStoreRaw = "nbd://example.org"; nbd2.path = NULL; - TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, - (&qcow2, &nbd2), EXP_PASS, - (&qcow2, &nbd2), EXP_PASS); + TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &nbd2), EXP_PASS); /* qed file */ testFileData qed = { @@ -947,12 +919,8 @@ mymain(void) .type = VIR_STORAGE_TYPE_FILE, .format = VIR_STORAGE_FILE_RAW, }; - TEST_CHAIN(absqed, VIR_STORAGE_FILE_QED, - (&qed, &raw), EXP_PASS, - (&qed, &raw), EXP_PASS); - TEST_CHAIN(absqed, VIR_STORAGE_FILE_AUTO, - (&qed_as_raw), EXP_PASS, - (&qed_as_raw), EXP_PASS); + TEST_CHAIN(absqed, VIR_STORAGE_FILE_QED, (&qed, &raw), EXP_PASS); + TEST_CHAIN(absqed, VIR_STORAGE_FILE_AUTO, (&qed_as_raw), EXP_PASS); /* directory */ testFileData dir = { @@ -965,15 +933,9 @@ mymain(void) .type = VIR_STORAGE_TYPE_DIR, .format = VIR_STORAGE_FILE_RAW, }; - TEST_CHAIN(absdir, VIR_STORAGE_FILE_RAW, - (&dir_as_raw), EXP_PASS, - (&dir_as_raw), EXP_PASS); - TEST_CHAIN(absdir, VIR_STORAGE_FILE_NONE, - (&dir), EXP_PASS, - (&dir), EXP_PASS); - TEST_CHAIN(absdir, VIR_STORAGE_FILE_DIR, - (&dir), EXP_PASS, - (&dir), EXP_PASS); + TEST_CHAIN(absdir, VIR_STORAGE_FILE_RAW, (&dir_as_raw), EXP_PASS); + TEST_CHAIN(absdir, VIR_STORAGE_FILE_NONE, (&dir), EXP_PASS); + TEST_CHAIN(absdir, VIR_STORAGE_FILE_DIR, (&dir), EXP_PASS); #ifdef HAVE_SYMLINK /* Rewrite qcow2 and wrap file to use backing names relative to a @@ -1011,7 +973,6 @@ mymain(void) raw.path = datadir "/sub/../sub/../raw"; raw.pathRel = "../raw"; TEST_CHAIN(abslink2, VIR_STORAGE_FILE_QCOW2, - (&link2, &link1, &raw), EXP_PASS, (&link2, &link1, &raw), EXP_PASS); #endif @@ -1024,9 +985,7 @@ mymain(void) qcow2.expBackingStoreRaw = "qcow2"; /* Behavior of an infinite loop chain */ - TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, - (&qcow2), EXP_WARN, - (&qcow2), EXP_WARN); + TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2), EXP_WARN); /* Rewrite wrap and qcow2 to be mutually-referential loop */ virCommandFree(cmd); @@ -1043,9 +1002,7 @@ mymain(void) qcow2.expBackingStoreRaw = "wrap"; /* Behavior of an infinite loop chain */ - TEST_CHAIN(abswrap, VIR_STORAGE_FILE_QCOW2, - (&wrap, &qcow2), EXP_WARN, - (&wrap, &qcow2), EXP_WARN); + TEST_CHAIN(abswrap, VIR_STORAGE_FILE_QCOW2, (&wrap, &qcow2), EXP_WARN); /* Rewrite qcow2 to use an rbd: protocol as backend */ virCommandFree(cmd); @@ -1063,9 +1020,7 @@ mymain(void) .format = VIR_STORAGE_FILE_RAW, .protocol = VIR_STORAGE_NET_PROTOCOL_RBD, }; - TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, - (&qcow2, &rbd1), EXP_PASS, - (&qcow2, &rbd1), EXP_PASS); + TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &rbd1), EXP_PASS); /* Rewrite qcow2 to use an rbd: protocol as backend */ virCommandFree(cmd); @@ -1085,9 +1040,7 @@ mymain(void) .secret = "asdf", .hostname = "example.com", }; - TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, - (&qcow2, &rbd2), EXP_PASS, - (&qcow2, &rbd2), EXP_PASS); + TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &rbd2), EXP_PASS); /* Rewrite wrap and qcow2 back to 3-deep chain, absolute backing */ -- 2.16.2

All callers pass 'false' now so it's no longer needed. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 4 +--- src/security/virt-aa-helper.c | 2 +- src/util/virstoragefile.c | 18 +++++++----------- src/util/virstoragefile.h | 1 - tests/virstoragetest.c | 2 +- 5 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d230ead359..d4771ff3f7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8061,9 +8061,7 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, qemuDomainGetImageIds(cfg, vm, src, disk->src, &uid, &gid); - if (virStorageFileGetMetadata(src, - uid, gid, false, - report_broken) < 0) + if (virStorageFileGetMetadata(src, uid, gid, report_broken) < 0) goto cleanup; for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 37fe36a329..971ee6733c 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -974,7 +974,7 @@ get_files(vahControl * ctl) * so that the open could be re-tried as that user:group. */ if (!virStorageSourceHasBacking(disk->src)) - virStorageFileGetMetadata(disk->src, -1, -1, false, false); + virStorageFileGetMetadata(disk->src, -1, -1, false); /* XXX passing ignoreOpenFailure = true to get back to the behavior * from before using virDomainDiskDefForeachPath. actually we should diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 62768af968..6ede542df6 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -4678,7 +4678,6 @@ static int virStorageFileGetMetadataRecurse(virStorageSourcePtr src, virStorageSourcePtr parent, uid_t uid, gid_t gid, - bool allow_probe, bool report_broken, virHashTablePtr cycle, unsigned int depth) @@ -4691,9 +4690,9 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, int backingFormat; int rv; - VIR_DEBUG("path=%s format=%d uid=%u gid=%u probe=%d", + VIR_DEBUG("path=%s format=%d uid=%u gid=%u", src->path, src->format, - (unsigned int)uid, (unsigned int)gid, allow_probe); + (unsigned int)uid, (unsigned int)gid); /* exit if we can't load information about the current image */ rv = virStorageFileSupportsBackingChainTraversal(src); @@ -4740,7 +4739,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, if (!(backingStore = virStorageSourceNewFromBacking(src))) goto cleanup; - if (backingFormat == VIR_STORAGE_FILE_AUTO && !allow_probe) + if (backingFormat == VIR_STORAGE_FILE_AUTO) backingStore->format = VIR_STORAGE_FILE_RAW; else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE) backingStore->format = VIR_STORAGE_FILE_AUTO; @@ -4749,7 +4748,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, if ((ret = virStorageFileGetMetadataRecurse(backingStore, parent, uid, gid, - allow_probe, report_broken, + report_broken, cycle, depth + 1)) < 0) { if (report_broken) goto cleanup; @@ -4802,12 +4801,11 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, int virStorageFileGetMetadata(virStorageSourcePtr src, uid_t uid, gid_t gid, - bool allow_probe, bool report_broken) { - VIR_DEBUG("path=%s format=%d uid=%u gid=%u probe=%d, report_broken=%d", + VIR_DEBUG("path=%s format=%d uid=%u gid=%u report_broken=%d", src->path, src->format, (unsigned int)uid, (unsigned int)gid, - allow_probe, report_broken); + report_broken); virHashTablePtr cycle = NULL; virStorageType actualType = virStorageSourceGetActualType(src); @@ -4819,14 +4817,12 @@ virStorageFileGetMetadata(virStorageSourcePtr src, if (src->format <= VIR_STORAGE_FILE_NONE) { if (actualType == VIR_STORAGE_TYPE_DIR) src->format = VIR_STORAGE_FILE_DIR; - else if (allow_probe) - src->format = VIR_STORAGE_FILE_AUTO; else src->format = VIR_STORAGE_FILE_RAW; } ret = virStorageFileGetMetadataRecurse(src, src, uid, gid, - allow_probe, report_broken, cycle, 1); + report_broken, cycle, 1); virHashFree(cycle); return ret; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 33fc853fcd..592e19bd7f 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -504,7 +504,6 @@ int virStorageFileSupportsAccess(const virStorageSource *src); int virStorageFileGetMetadata(virStorageSourcePtr src, uid_t uid, gid_t gid, - bool allow_probe, bool report_broken) ATTRIBUTE_NONNULL(1); diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 2236351309..ad92f3b875 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -117,7 +117,7 @@ testStorageFileGetMetadata(const char *path, if (VIR_STRDUP(ret->path, path) < 0) goto error; - if (virStorageFileGetMetadata(ret, uid, gid, false, false) < 0) + if (virStorageFileGetMetadata(ret, uid, gid, false) < 0) goto error; return ret; -- 2.16.2