[libvirt] [PATCH] spec: Remove qemu-img dependency from storage driver

Both qemu-img and its stripped down version qcow-create are searched for dynamically with virFindFileInPath, functions using them report errors when the required binary is not present, and we don't compile in a fixed path detected by configure anymore. So I don't see a reason for keeping the dependencies for our storage driver. Moreover, it's weired when the dependencies (of the storage driver) change depending on wheter a particular hypervisor driver (xen/qemu) is enabled or not. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- libvirt.spec.in | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index bda28e7..dbeb62a 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -530,15 +530,6 @@ BuildRequires: PolicyKit-devel >= 0.6 # For mount/umount in FS driver BuildRequires: util-linux %endif -%if %{with_qemu} -# From QEMU RPMs -BuildRequires: /usr/bin/qemu-img -%else - %if %{with_xen} -# From Xen RPMs -BuildRequires: /usr/sbin/qcow-create - %endif -%endif %if %{with_storage_lvm} # For LVM drivers BuildRequires: lvm2 @@ -871,15 +862,6 @@ Requires: device-mapper # For Sheepdog support Requires: sheepdog %endif - %if %{with_qemu} -# From QEMU RPMs -Requires: /usr/bin/qemu-img - %else - %if %{with_xen} -# From Xen RPMs -Requires: /usr/sbin/qcow-create - %endif - %endif %description daemon-driver-storage The storage driver plugin for the libvirtd daemon, providing -- 2.2.0

On Wed, Dec 03, 2014 at 04:52:25PM +0100, Jiri Denemark wrote:
Both qemu-img and its stripped down version qcow-create are searched for dynamically with virFindFileInPath, functions using them report errors when the required binary is not present, and we don't compile in a fixed path detected by configure anymore. So I don't see a reason for keeping the dependencies for our storage driver. Moreover, it's weired when the dependencies (of the storage driver) change depending on wheter a particular hypervisor driver (xen/qemu) is enabled or not.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- libvirt.spec.in | 18 ------------------ 1 file changed, 18 deletions(-)
Even if using the Xen hypervisor, you can use the qemu-img binary for creating volumes. Since the qemu-img binary is in a separate RPM from the main QEMU emulator binaries, it makes sense to depend on qemu-img if we're enabling QEMU support in libvirt. Similarly the LXC driver can make use of qemu-img for creating images that are then used via qemu-nbd. So I think our current Requires deps are actually correct here - they ensure that the qemu-img binary is always pulled into the host, no matter what daemon-driver-XXX is installed. IMHO only the BuildRequires are redundant.
diff --git a/libvirt.spec.in b/libvirt.spec.in index bda28e7..dbeb62a 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -530,15 +530,6 @@ BuildRequires: PolicyKit-devel >= 0.6 # For mount/umount in FS driver BuildRequires: util-linux %endif -%if %{with_qemu} -# From QEMU RPMs -BuildRequires: /usr/bin/qemu-img -%else - %if %{with_xen} -# From Xen RPMs -BuildRequires: /usr/sbin/qcow-create - %endif -%endif %if %{with_storage_lvm} # For LVM drivers BuildRequires: lvm2 @@ -871,15 +862,6 @@ Requires: device-mapper # For Sheepdog support Requires: sheepdog %endif - %if %{with_qemu} -# From QEMU RPMs -Requires: /usr/bin/qemu-img - %else - %if %{with_xen} -# From Xen RPMs -Requires: /usr/sbin/qcow-create - %endif - %endif
%description daemon-driver-storage The storage driver plugin for the libvirtd daemon, providing
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Wed, Dec 03, 2014 at 15:58:45 +0000, Daniel Berrange wrote:
On Wed, Dec 03, 2014 at 04:52:25PM +0100, Jiri Denemark wrote:
Both qemu-img and its stripped down version qcow-create are searched for dynamically with virFindFileInPath, functions using them report errors when the required binary is not present, and we don't compile in a fixed path detected by configure anymore. So I don't see a reason for keeping the dependencies for our storage driver. Moreover, it's weired when the dependencies (of the storage driver) change depending on wheter a particular hypervisor driver (xen/qemu) is enabled or not.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- libvirt.spec.in | 18 ------------------ 1 file changed, 18 deletions(-)
Even if using the Xen hypervisor, you can use the qemu-img binary for creating volumes. Since the qemu-img binary is in a separate RPM from the main QEMU emulator binaries, it makes sense to depend on qemu-img if we're enabling QEMU support in libvirt. Similarly the LXC driver can make use of qemu-img for creating images that are then used via qemu-nbd.
So I think our current Requires deps are actually correct here - they ensure that the qemu-img binary is always pulled into the host, no matter what daemon-driver-XXX is installed.
But it still feels wrong. A choice of what hypervisor driver is enabled should not influence dependencies of a storage driver. If we want the storage driver to depend on qemu-img, I think the dependency should be enabled by a separate switch, so that one can enable qemu-img support without enabling qemu driver. Or am I just over-engineering it based on my confusion why storage deps change depending on enabled HV drivers? Jirka

On Thu, Dec 04, 2014 at 02:42:31PM +0100, Jiri Denemark wrote:
On Wed, Dec 03, 2014 at 15:58:45 +0000, Daniel Berrange wrote:
On Wed, Dec 03, 2014 at 04:52:25PM +0100, Jiri Denemark wrote:
Both qemu-img and its stripped down version qcow-create are searched for dynamically with virFindFileInPath, functions using them report errors when the required binary is not present, and we don't compile in a fixed path detected by configure anymore. So I don't see a reason for keeping the dependencies for our storage driver. Moreover, it's weired when the dependencies (of the storage driver) change depending on wheter a particular hypervisor driver (xen/qemu) is enabled or not.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- libvirt.spec.in | 18 ------------------ 1 file changed, 18 deletions(-)
Even if using the Xen hypervisor, you can use the qemu-img binary for creating volumes. Since the qemu-img binary is in a separate RPM from the main QEMU emulator binaries, it makes sense to depend on qemu-img if we're enabling QEMU support in libvirt. Similarly the LXC driver can make use of qemu-img for creating images that are then used via qemu-nbd.
So I think our current Requires deps are actually correct here - they ensure that the qemu-img binary is always pulled into the host, no matter what daemon-driver-XXX is installed.
But it still feels wrong. A choice of what hypervisor driver is enabled should not influence dependencies of a storage driver. If we want the storage driver to depend on qemu-img, I think the dependency should be enabled by a separate switch, so that one can enable qemu-img support without enabling qemu driver. Or am I just over-engineering it based on my confusion why storage deps change depending on enabled HV drivers?
I can see the argument that we should use a separate conditional for enabling qemu-img, because it is possible that there are architectures where qemu-img is built, but the qemu emulators are not built. So we would wnat the dep on qemu-img, even if qemu was disabled in libvirt. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 12/03/14 16:52, Jiri Denemark wrote:
Both qemu-img and its stripped down version qcow-create are searched for dynamically with virFindFileInPath, functions using them report errors when the required binary is not present, and we don't compile in a fixed path detected by configure anymore. So I don't see a reason for keeping the dependencies for our storage driver. Moreover, it's weired when the
s/weired/weird/
dependencies (of the storage driver) change depending on wheter a
s/wheter/whether/
particular hypervisor driver (xen/qemu) is enabled or not.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- libvirt.spec.in | 18 ------------------ 1 file changed, 18 deletions(-)
ACK Peter
participants (3)
-
Daniel P. Berrange
-
Jiri Denemark
-
Peter Krempa