Bug 1518517

Some kind of fall-out from the qemu locking changes in libvirt
at a guess ...  I'll have to have a proper look at this when I
get back.

The error message hints that 'sdb' has a <shareable/> tag but is a qcow2 or other non-raw disk. Since qcow2 would be corrupted if concurrent writes would be attempted, libvirt rejects such configuration.

Note that even if libvirt did not reject it, qemu would. The sharing enabled by libvirt does not override all write-protections, only those which still guarantee working configuration.

With the proviso that I've not looked at this in any detail yet
and won't be able to until next week, the XML for this disk is:

<disk type="file" device="disk">
      <source file="/tmp/libguestfscQ4OMx/overlay1.qcow2"/>
      <target dev="sdb" bus="scsi"/>
      <driver name="qemu" type="qcow2" cache="unsafe"/>
      <address type="drive" controller="0" bus="0" target="1" unit="0"/>
      <shareable/>
    </disk>

where the qcow2 is only used by the current process.  The qcow2's
backing file is /var/tmp/.guestfs-0/appliance.d/root which is *raw*
format and *is* (possibly) shared with other processes.

So I think libvirt is wrong here.

No. With <shareable/> you declare that you want to share writes on the top level image which could possibly corrupt metadata of the qcow2 image, so using <shareable/> in this case is wrong.

I have to check whether current qemu as we configure it allows the libguestfs use case described here and I'll see what we can do in that case.

Note that libvirt will not try to use file.locking=off since it removes protections which make sense (e.g. two processes trying to write the same qcow2 file)

Well I guess we've been doing <shareable/> since forever so
the libvirt XML has changed semantics.  What should the XML
look like for this case?

You'll need to omit the <shareable/> element. Cache setting probably should be set to 'none' so that any changes are properly propagated, but since you did not use it using 'unsafe' may still be okay.

Note that <shareable/> now puts 'share-rw=on' onto the command line for the qemu device, so even if libvirt would not report the error qemu would.

As said. I need to make sure that qemu in such configuration will be okay with opening the backing file of the overlay as read-only even if another instance uses it in read-write mode, since we are not wanting to do file.locking=off in this case.

Note the backing file is really read-only.  If anything else had it
open read-write then that would be a bug.

So I tried configuration, where one qemu had an image open read-write and then I've created a qcow2 overlay on top of it and tired to start a second qemu with it. I've got qemu complaining that it's not allowed:

2017-11-29T16:35:59.350071Z qemu-system-x86_64: -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0xa,drive=drive-virtio-disk0,id=virtio-disk0: Failed to get shared "write" lock
Is another process using the image?

While the configuration will not result in data corruption of the original image the second qemu may still get garbage though. Is such configuration expected in libguestfs?

The backing file in this case (/var/tmp/.guestfs-0/appliance.d/root)
is the pristine appliance and nothing should ever be writing to it.
Even in the case where it gets rebuilt it is replaced atomically
without affecting the version used by current users.

So in this case libguestfs should not put <shareable> on the commandline. I'll move this bug to libguestfs.

If you think that you might need the situation I've described in Comment 10, we will need to wire-up a way to use the locking=off infrastructure. In such case please open a bug on libvirt tracking that issue. A new option for that is necessary as sharing a backing file which is written to (especially if it's qcow2) is that even reading can result in corrupted metadata in the cache, and thus the "read-only" process reading total garbage.

I don't think it is reasonable to require libguestfs to remove <shareable/> because that's what they've used for ages, and we should allow it to continue working, otherwise its an effective API break. That said, it is possibly reasonable to require additional use of a flag to turn off strict locking.

Well, there's no other possibility. If you want to have shared disks, the <shareable/> flag for a read write disk will need to add the share-rw parameter to the command line of qemu. Otherwise shareable disks will not work due to qemu image locking.

And even if we'd not report that shareable does not work with qcow2 qemu would report it. So unless qemu disables image locking by default, there's no way to appease both users of <shareable/> disks which are in fact shareable and libguestfs which does not in fact share the top level image.

Okay, technically we can do it. Qemu does not actually report an error if share-rw=on is used with qcow2. That means that the fix for https://bugzilla.redhat.com/show_bug.cgi?id=1511480 will need to be reverted without any replacement making the bug invalid.

Before we do that, I wonder if rjones can remind us why libguestfs uses <shareable/> right now, given that the libguest qcow2 overlay doesn't actually need shared right facility.

I vaguely recall it might have been something related to SELinux labelling, but damned if I can recall what.  If it was SELinux, then an alternative might be to rely on the per-disk <seclabel> config with a write-exclusive disk, which could work with both old & new libvirt.

Looking in the git history, it seems use of <shareable/> was added waaaay back in

commit aa9e0057b19e29f76c9a81f9aebeeb1cb5bf1fdb (HEAD)
Author: Richard W.M. Jones <rjones>
Date:   Fri Sep 14 15:50:38 2012 +0100

    launch: libvirt: Mark appliance disk as <shareable/> (thanks Dan Berrange).
    
    This is a fix for multiprogramming: If two instances of libguestfs
    share the same appliance disk, then libvirt would unlabel the disk
    when one of the instances closes the handle, resulting in the other
    qemu being unable to continue accessing the appliance.
    
    Adding the flag makes libvirt understand that the disk is shared so it
    doesn't do this, and it apparently handles locking correctly too if we
    were using sanlock.


If i'm interpreting the code correctly, at that point in time, libguestfs did indeed have a single disk image for the appliance that was shared between all instances.

At some point, it appears libguestfs changed such that the master appliance disk image was not used directly, but instead a temporary qcow2 overlay was added. The use of the <shareable/> flag continued, however, despite this overlay being private to each QEMU instance.

If that interpretation is correct, then I think libguestfs could simply remove the use of the <shareable/> flag here.

We can remove the <shareable/> flag from the appliance disk per
discussion above.

However I'll note it was added because of SELinux labels, so I
hope that will still work if the backing disk needs to be labelled.

If we have any SELinux labelling problems, then a per-disk <seclabel> tag can most likely solve them these days.

Fixed upstream with
https://github.com/libguestfs/libguestfs/commit/614f680dc67d98d16eb21a131cc43db24d08b758
which is in libguestfs >= 1.37.35.

*** Bug 1523104 has been marked as a duplicate of this bug. ***

Reproduce the bug with builds:
virt-v2v-1.36.10-2.el7.x86_64
libguestfs-1.36.10-2.el7.x86_64
libvirt-3.9.0-5.el7.x86_64
qemu-kvm-rhev-2.10.0-11.el7.x86_64

Steps:
1.Make sure libguestfs_backend environment is empty
# echo $LIBGUESTFS_BACKEND
nothing

2.Convert a guest to rhv by virt-v2v and conversion will be failed with env error as below:
# virt-v2v sles12sp1 -o rhv -os 10.73.131.93:/home/nfs_export 
[   0.0] Opening the source -i libvirt sles12sp1
[   0.0] Creating an overlay to protect the source from being modified
[   0.2] Initializing the target -o rhv -os 10.73.131.93:/home/nfs_export
[   0.5] Opening the overlay
virt-v2v: error: libguestfs error: could not create appliance through 
libvirt.

Try running qemu directly without libvirt using this environment variable:
export LIBGUESTFS_BACKEND=direct

Original error from libvirt: unsupported configuration: shared access for 
disk 'sdb' requires use of supported storage format [code=67 int1=-1]

If reporting bugs, run virt-v2v with debugging enabled and include the 
complete output:

  virt-v2v -v -x [...]

3.Check overlay1.qcow2's flag info in debug log of conversion,there is <shareable/> flag in the appliance disk
.....
 <disk type="file" device="disk">     
   <source file="/tmp/libguestfs6kEPJp/overlay1.qcow2"/>    
     <target dev="sdb" bus="scsi"/>
      <driver name="qemu" type="qcow2" cache="unsafe"/>
     <address type="drive" controller="0" bus="0" target="1" unit="0"/>      
   <shareable/>
 </disk>
.....


Update virt-v2v builds to below and verify the bug
virt-v2v-1.36.10-3.el7.x86_64
libguestfs-1.36.10-3.el7.x86_64
libvirt-3.9.0-5.el7.x86_64
qemu-kvm-rhev-2.10.0-11.el7.x86_64

Steps:
1.Make sure libguestfs_backend environment is empty
# echo $LIBGUESTFS_BACKEND
nothing

2.Convert a guest to rhv by virt-v2v and conversion could be finished successfully
# virt-v2v sles12sp1 -o rhv -os 10.73.131.93:/home/nfs_export 
[   0.0] Opening the source -i libvirt sles12sp1
[   0.0] Creating an overlay to protect the source from being modified
[   0.5] Initializing the target -o rhv -os 10.73.131.93:/home/nfs_export
[   0.6] Opening the overlay
[  12.0] Inspecting the overlay
[  25.5] Checking for sufficient free disk space in the guest
[  25.5] Estimating space required on target for each disk
[  25.5] Converting SUSE Linux Enterprise Server 12 SP1 to run on KVM
virt-v2v: warning: /files/boot/grub2/device.map/hd0 references unknown 
device "sda".  You may have to fix this entry manually after conversion.
virt-v2v: This guest has virtio drivers installed.
[  48.5] Mapping filesystem data to avoid copying unused and blank areas
[  49.3] Closing the overlay
[  49.6] Checking if the guest needs BIOS or UEFI to boot
[  49.6] Assigning disks to buses
[  49.6] Copying disk 1/1 to /tmp/v2v.12xKQc/bdf9c90b-e6f0-439c-aa9f-6305fd5fad7e/images/e2061054-9db6-4740-9976-5484e9e21860/c06e5af4-fe5a-4dd7-abc5-098a202a2887 (qcow2)
    (100.00/100%)
[ 373.5] Creating output metadata
[ 373.6] Finishing off
You have new mail in /var/spool/mail/root

3.Check overlay1.qcow2's flag info in debug log of conversion
....
 <disk type="file" device="disk">  
   <source file="/tmp/libguestfsuD9bEH/overlay1.qcow2"/>  
    <target dev="sdb" bus="scsi"/>\n    
   <driver name="qemu" type="qcow2" cache="unsafe"/>     
    <address type="drive" controller="0" bus="0" target="1" unit="0"/>
 </disk> 
....


Result:
The v2v conversion could be finished without error when libguestfs_backend environment is empty and <shareable/> flag has been removed from the appliance disk, so the bug has been fixed with latest v2v builds,then move this bug from ON_QA to VERIFIED

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2018:0677