Bug 1990268 - No ability to change iso with virtio drivers when installing a virtual machine with windows
Summary: No ability to change iso with virtio drivers when installing a virtual machin...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: vdsm
Classification: oVirt
Component: Core
Version: 4.40.70
Hardware: Unspecified
OS: Unspecified
unspecified
medium
Target Milestone: ovirt-4.4.9
: 4.40.90.2
Assignee: Vojtech Juranek
QA Contact: Evelina Shames
URL:
Whiteboard:
: 1998873 2008438 (view as bug list)
Depends On: 2003644
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-08-05 07:18 UTC by Patrick
Modified: 2021-12-23 16:34 UTC (History)
10 users (show)

Fixed In Version: vdsm-4.40.90.2
Doc Type: Bug Fix
Doc Text:
Cause: Block-based CDs use wrong driver in XML description. Consequence: Recent QEMU version checks is and now, when changing CD to block-based image, QEMU fails with "'file' driver requires '/path/to/image' to be a regular file". Fix: Use correct 'block' driver for block-based CDs. Result: Loading and changing block-based CDs work without any issue.
Clone Of:
Environment:
Last Closed: 2021-11-20 07:58:31 UTC
oVirt Team: Storage
Embargoed:
pm-rhel: ovirt-4.4+


Attachments (Terms of Use)
Error of iso changing (16.28 KB, image/png)
2021-08-05 07:18 UTC, Patrick
patrick.lomakin: review?
Details
vdsm log from host01 (931.18 KB, text/plain)
2021-08-07 16:39 UTC, Patrick
patrick.lomakin: review?
Details


Links
System ID Private Priority Status Summary Last Updated
Red Hat Issue Tracker RHV-42966 0 None None None 2021-09-21 11:25:30 UTC
oVirt gerrit 116152 0 master MERGED virt: user proper disk type when changing CD 2021-10-05 15:02:12 UTC
oVirt gerrit 116849 0 master ABANDONED virt: remove startup policy before changing CD 2021-10-05 08:46:18 UTC
oVirt gerrit 116941 0 master MERGED spec: bump libvirt version 2021-10-05 15:02:10 UTC
oVirt gerrit 116976 0 ovirt-4.4.z MERGED spec: bump libvirt version 2021-10-06 09:31:00 UTC
oVirt gerrit 116977 0 ovirt-4.4.z MERGED virt: user proper disk type when changing CD 2021-10-06 09:31:07 UTC
oVirt gerrit 116981 0 ovirt-4.4.z MERGED tests: add support for storage type to fake IRS 2021-10-06 09:31:03 UTC

Description Patrick 2021-08-05 07:18:43 UTC
Created attachment 1811119 [details]
Error of iso changing

Description of problem:
After upgrading from version 4.6 to 4.7, a previously unknown error occurs when changing the iso-image in the cd-rom. After creating a new virtual machine with Windows 10, I run it through the "Run Once" command and select "Attach CD" with the Windows 10 image and also select "Attach Windows guest tools CD". After that, I start the virtual machine and get to the point of installing the driver for the virtual drive. After I select "Change CD" and select the iso image with the virtio drivers, this error appears: 

Error while executing action Change CD: Failed to perform "Change CD" operation, CD might be still in use by the VM.
Please try to manually detach the CD from withing the VM:
1. Log in to the VM
2 For Linux VMs, un-mount the CD using umount command;
For Windows VMs, right click on the CD drive and click 'Eject';

No matter how I try to mount or unmount the CD, the error stays on.

Version-Release number of selected component (if applicable): 4.4.7.6


How reproducible: 100%


Steps to Reproduce:
1. Run Once new VM and select "Attach CD" with the Windows 10 image and also select "Attach Windows guest tools CD"
2. Select "Change CD"
3. Select the iso image with the virtio drivers

Actual results:
 Get an error: Error while executing action Change CD: Failed to perform "Change CD" operation, CD might be still in use by the VM.

Expected results:

Mount selected iso to the VM

Additional info:

Comment 1 RHEL Program Management 2021-08-05 12:31:18 UTC
The documentation text flag should only be set after 'doc text' field is provided. Please provide the documentation text and set the flag to '?' again.

Comment 2 Michal Skrivanek 2021-08-06 06:47:54 UTC
do you have /var/log/vdsm.log from the host where the VM running handy? Please attach and indicate the relevant time

Comment 3 Patrick 2021-08-07 16:39:58 UTC
Created attachment 1811900 [details]
vdsm log from host01

Relevant Time: about 19:20:00 UTC

Comment 4 Arik 2021-08-08 13:57:31 UTC
Thanks for providing the VDSM log

It fails on libvirt with:
libvirt.libvirtError: internal error: unable to execute QEMU command 'blockdev-add': 'file' driver requires '/rhev/data-center/mnt/blockSD/e8cc92bc-06b6-4081-8a6d-5e18d384e77b/images/f1a3a3c4-5975-41e4-9c5b-64177e0b0dc2/ea512e1b-de4e-47a9-88ca-7b5e879aa398' to be a regular file

We really point to a volume on a block storage with "<source file=..>":
2021-08-07 19:22:10,566+0300 INFO  (jsonrpc/4) [virt.vm] (vmId='17cbaeb2-5fcc-4af1-9ee8-edf65accb1b9') Updating disk device using XML: <?xml version='1.0' encoding='utf-8'?>
<disk device="cdrom" type="file"><source file="/rhev/data-center/mnt/blockSD/e8cc92bc-06b6-4081-8a6d-5e18d384e77b/images/f1a3a3c4-5975-41e4-9c5b-64177e0b0dc2/ea512e1b-de4e-47a9-88ca-7b5e879aa398" /><target bus="sata" dev="sdc" /></disk> (vm:4981)

I assume the problem is in:
https://github.com/oVirt/vdsm/blob/v4.40.70.6/lib/vdsm/virt/vm.py#L5005-L5006

Moving this bz since having ISOs on block storage was implemented by the storage team

Comment 5 Eyal Shenitzky 2021-08-09 14:17:14 UTC
Vojtech, can you please have a look if this issue already resolved by the 'change CD' fix?

Comment 6 Vojtech Juranek 2021-08-10 13:32:23 UTC
(In reply to Eyal Shenitzky from comment #5)
> Vojtech, can you please have a look if this issue already resolved by the
> 'change CD' fix?

Although I'm not able to reproduce this issue, this is obviously a bug in new CD change implementation (before that it wasn't possible to use CD on block storage at all).

Comment 7 Vojtech Juranek 2021-08-30 06:55:49 UTC
*** Bug 1998873 has been marked as a duplicate of this bug. ***

Comment 8 Michal Skrivanek 2021-09-17 12:38:46 UTC
With bug 2003644 in 8.5 this is just a TestOnly, no?

Removing the functionality of optional cdrom should be properly deprecated and removed in larger version only, if that's what we really want to do. We can also just leave it as is, with a note that it doesn't work on block SDs

Comment 9 Vojtech Juranek 2021-09-17 13:36:42 UTC
(In reply to Michal Skrivanek from comment #8)
> With bug 2003644 in 8.5 this is just a TestOnly, no?

BZ #2003644 was discovered when I test fix for this bug. Fix for BZ #2003644 won't solve this issue.

> Removing the functionality of optional cdrom should be properly deprecated
> and removed in larger version only, if that's what we really want to do.

AFAICT removing startupPolicy affect only libvirt, if the backing file for CD is not present, vdsm would fail anyway, so from user perspective there's no change.

Comment 10 Vojtech Juranek 2021-09-21 11:53:42 UTC
> > Removing the functionality of optional cdrom should be properly deprecated
> > and removed in larger version only, if that's what we really want to do.
> 
> AFAICT removing startupPolicy affect only libvirt, if the backing file for
> CD is not present, vdsm would fail anyway, so from user perspective there's
> no change.

This is not correct and unfortunately you were right - if the CD backing file is e.g. missing, removing startupPolicy="optional" would cause that VM won't boot. It looks like libvirt removes the disk source when it's missing and has startupPolicy="optional" so nothing fails. With startupPolicy="optional" VM with attach CD which is not available loks like this after the boot:

    <disk type='file' device='cdrom'>
      <driver name='qemu' error_policy='report'/>
      <target dev='sdc' bus='sata'/>
      <readonly/>
      <alias name='ua-a4e16e85-bab1-4b14-8fb4-6df5faf466af'/>
      <address type='drive' controller='0' bus='0' target='0' unit='2'/>
    </disk>

while when the startupPolicy="optional" is removed, it's

         <disk device="cdrom" snapshot="no" type="file">
            <driver error_policy="report" name="qemu" type="raw" />
            <source file="/rhev/data-center/19016d46-17b4-11ec-9517-5254003aa1f8/eb405d82-0cd7-45c0-bba0-3795a34a6e5b/images/e979ebc2-911d-40b8-b727-79ae28ab57df/77df79dd-5b2c-42ad-8116-e83d4b3f8951">
                <seclabel model="dac" relabel="no" type="none" />
            </source>
            <target bus="sata" dev="sdc" />
            <readonly />
            <alias name="ua-a4e16e85-bab1-4b14-8fb4-6df5faf466af" />
            <address bus="0" controller="0" target="0" type="drive" unit="2" />
        </disk>

and VM fails with 

    2021-09-21 07:16:34,252-0400 ERROR (vm/651faf06) [virt.vm] (vmId='651faf06-9dd3-41a5-94f6-ef126a933923') The vm start process failed (vm:992)
Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/vdsm/virt/vm.py", line 919, in _startUnderlyingVm
    self._run()
  File "/usr/lib/python3.6/site-packages/vdsm/virt/vm.py", line 2967, in _run
    dom.createWithFlags(flags)
  File "/usr/lib/python3.6/site-packages/vdsm/common/libvirtconnection.py", line 131, in wrapper
    ret = f(*args, **kwargs)
  File "/usr/lib/python3.6/site-packages/vdsm/common/function.py", line 94, in wrapper
    return func(inst, *args, **kwargs)
  File "/usr/lib64/python3.6/site-packages/libvirt.py", line 1385, in createWithFlags
    raise libvirtError('virDomainCreateWithFlags() failed')
libvirt.libvirtError: Cannot access storage file '/rhev/data-center/19016d46-17b4-11ec-9517-5254003aa1f8/eb405d82-0cd7-45c0-bba0-3795a34a6e5b/images/e979ebc2-911d-40b8-b727-79ae28ab57df/77df79dd-5b2c-42ad-8116-e83d4b3f8951': No such file or directory

So we will have to find another workaround or wait for libvirt fir fix for BZ #2003644.

Comment 11 Arik 2021-09-29 07:06:21 UTC
*** Bug 2008438 has been marked as a duplicate of this bug. ***

Comment 12 Nir Soffer 2021-09-29 12:52:44 UTC
(In reply to Vojtech Juranek from comment #10)
> > > Removing the functionality of optional cdrom should be properly deprecated
> > > and removed in larger version only, if that's what we really want to do.
> > 
> > AFAICT removing startupPolicy affect only libvirt, if the backing file for
> > CD is not present, vdsm would fail anyway, so from user perspective there's
> > no change.
> 
> This is not correct and unfortunately you were right - if the CD backing
> file is e.g. missing, removing startupPolicy="optional" would cause that VM
> won't boot. 

This is an issue only with the legacy ISO domain which is deprecated in 4.4.

With ISO domain oVirt does not manage the images on the domain, so it make sense
to make the cdrom optional. A user can remove an image without updating the VM
configuartion.

With data domain oVirt manages the disks and there is no way to remove a disk used
by a VM. You must remove the disk from the VM.

If there is an engine bug allowing removing an ISO disk used by a VM, the VM will
fail to start on the next boot, since preparing the image will fail.

I think this should be fix by:

1. Change engine to use startupPolicy only for ISO files on ISO domain
2. Change vdsm to use startupPolicy only when changing CD to ISO domain
3. Get this trivial fix in libvirt to avoid the failure when changing CD:
   https://listman.redhat.com/archives/libvir-list/2021-September/msg00283.html

Next release will require 8.5, and there is no reason not to have this fix in libvirt.
We should not add complex workarounds in vdsm when we have the real fix in libvirt.

Comment 13 Vojtech Juranek 2021-09-29 13:36:57 UTC
> 
> Next release will require 8.5, and there is no reason not to have this fix
> in libvirt.
> We should not add complex workarounds in vdsm when we have the real fix in
> libvirt.

I guess we don't have to add any extra handling for ISO domain then and alway use startupPolicy (i.e. keep current code as it is).

Comment 14 Nir Soffer 2021-09-29 13:43:11 UTC
We may need to change change cd from data domain to iso domain - currently we call:

4981     def _create_disk_xml(self, vm_dev, path, device, iface, type):
4982         disk_elem = vmxml.Element('disk', type=type, device=vm_dev)
4983         disk_elem.appendChildWithArgs('source', file=path)
4984 
4985         target = {'dev': device}
4986         if iface:
4987             target['bus'] = iface
4988 
4989         disk_elem.appendChildWithArgs('target', **target)

This creates XML without startupPolicy, which is not compatible with the XML
created on engien.

But this is not related to this bug, we can file another bug for this.

We certainly need the pending patches adding missing tests.

Comment 16 RHEL Program Management 2021-10-06 07:20:58 UTC
Target release should be placed once a package build is known to fix a issue. Since this bug is not modified, the target version has been reset. Please use target milestone to plan a fix for a oVirt release.

Comment 17 RHEL Program Management 2021-10-06 07:30:39 UTC
Target release should be placed once a package build is known to fix a issue. Since this bug is not modified, the target version has been reset. Please use target milestone to plan a fix for a oVirt release.

Comment 19 Sandro Bonazzola 2021-11-20 07:58:31 UTC
$ git tag --contains 56aaf03e3357d769516787c8aff8c0ed3df9e0a4
v4.40.100.1
v4.40.90.2
v4.40.90.3
v4.40.90.4


This bugzilla is included in oVirt 4.4.9 release, published on October 20th 2021.

Since the problem described in this bug report should be resolved in oVirt 4.4.9 release, it has been closed with a resolution of CURRENT RELEASE.

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


Note You need to log in before you can comment on or make changes to this bug.