Bug 1342999 - 'cache=x' cannot work with empty cdrom
Summary: 'cache=x' cannot work with empty cdrom
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: qemu-kvm-rhev
Version: 7.3
Hardware: x86_64
OS: Linux
unspecified
medium
Target Milestone: rc
: ---
Assignee: John Snow
QA Contact: FuXiangChun
URL:
Whiteboard:
Depends On:
Blocks: 1377321
TreeView+ depends on / blocked
 
Reported: 2016-06-06 10:05 UTC by Pei Zhang
Modified: 2017-01-20 07:36 UTC (History)
22 users (show)

Fixed In Version: qemu-kvm-rhev-2.6.0-26.el7
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1377321 (view as bug list)
Environment:
Last Closed: 2016-11-07 21:14:29 UTC
Target Upstream Version:


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2016:2673 normal SHIPPED_LIVE qemu-kvm-rhev bug fix and enhancement update 2016-11-08 01:06:13 UTC

Description Pei Zhang 2016-06-06 10:05:17 UTC
Description of problem:
Start qemu with an empty cdrom which with 'cache' option, qemu will prompt errors. 

Version-Release number of selected component (if applicable):
kernel: 3.10.0-394.el7.x86_64
qemu-kvm-rhev: qemu-kvm-rhev-2.6.0-3.el7.x86_64

How reproducible:
100%

Steps to Reproduce:
1. Start qemu with an empty cdrom, specify cache=none, qemu prompt errors:
# /usr/libexec/qemu-kvm \
-device virtio-scsi-pci,id=virtio_scsi_pci0,bus=pci.0,addr=06,disable-legacy=off,disable-modern=on \
-drive id=drive_cd1,if=none,media=cdrom,aio=native,snapshot=off,cache=none \
-device scsi-cd,id=cd1,drive=drive_cd1,bootindex=1,serial=X1CdkyW \

qemu-kvm: -drive id=drive_cd1,if=none,media=cdrom,aio=native,snapshot=off,cache=none: Must specify either driver or file

2. Libvirt hit same issue
<disk type='file' device='cdrom'>
   <driver name='qemu' type='raw' cache='none'/>
   <target dev='sdb' bus='scsi'/>
   <readonly/>
   <address type='drive' controller='0' bus='0' target='1' unit='0'/>
</disk>

# virsh start nfv_config2
error: Failed to start domain nfv_config2
error: internal error: process exited while connecting to monitor: 2016-06-06T08:24:50.944631Z qemu-kvm: -drive if=none,id=drive-scsi0-0-1-0,readonly=on,cache=none: Must specify either driver or file


Actual results:
qemu-kvm: -drive if=none,id=drive-scsi0-0-1-0,readonly=on,cache=none: Must specify either driver or file

Expected results:
1. 'cache' option should work well with empty cdrom.
2. Or qemu should prompt more clear log such as 'Don't specify cache option to an empty cdrom'.

Additional info:
it's valid in rhel7.2. So if it's a bug, it will be a regression.

Comment 1 Pei Zhang 2016-06-06 10:16:43 UTC
Many thanks to fam, he gave me many help about this issue, 

IRC with fam:
fam: I don't think it is a bug, it is more like an invalid configuration
fam: but you can file a BZ if you disagree with me, the dev who gets assigned can make a decision
pezhang: OK.  If insert an iso to the empty cdrom, I think the cache will become useful. Thank you very much for your advice.   I will file a BZ.


So I file this bug for more ideas. if you agree with fam, please feel free to close it. Thank you in advance.

Comment 4 John Snow 2016-06-23 23:29:52 UTC
As much as it bothers me as well, this will be CLOSED NOTABUG.

The cache=x option is a shorthand for three other cache options:

#define BDRV_OPT_CACHE_WB       "cache.writeback"
#define BDRV_OPT_CACHE_DIRECT   "cache.direct"
#define BDRV_OPT_CACHE_NO_FLUSH "cache.no-flush"


The writeback option may be applied to an empty drive, but direct and no-flush may not be. By using the cache=x shorthand, you (inadvertently) set the other two options as well.

These options are then passed to blk_new_open, which rightly want a file to open.

Further, we are moving away from applying cache options to empty drives, so we will not be implementing the ability to apply cache options to empty drives (upstream).

The recommended workflow in the future is to use blockdev-add and specify cache options there (to open a new ISO) and then use x-blockdev-insert-medium to insert that disc into the drive.

We will continue to support (upstream) persistent cache options when using the legacy QMP/HMP commands "eject" and "change," but there are no plans to support persistent cache options on the CLI or through the newer QMP interface.

"OK, that's unfortunate, but the error message is very bad."

I know, and it's not easy to fix. It would definitely be an RFE, and not appropriate for qemu-kvm-rhev in 7.3.


As such, I will be closing this.


If anyone should by chance happen across this bug in the future and has strong feelings about the usability of the error messages, please file a bug for the upstream QEMU bug tracker (https://bugs.launchpad.net/qemu).

Thank you,
--John Snow

Comment 5 Fam Zheng 2016-06-24 00:13:28 UTC
I think there is a valid configuration from virt-manager and libvirt that triggers this error and therefore QEMU fails to start.

In virt-manager, add an CDROM device, leave the path text box blank, and select a cache mode for it. The result xml in libvirt looks like:

    <disk type='file' device='cdrom'>
      <driver name='qemu' type='raw' cache='unsafe'/>
      <target dev='hda' bus='ide'/>
      <readonly/>
      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
    </disk>

But QEMU refuses to start it:

$ virsh start rhel7.2
error: Failed to start domain rhel7.2
error: internal error: process exited while connecting to monitor: 2016-06-24T00:11:20.660157Z qemu-system-x86_64: -drive if=none,id=drive-ide0-0-0,readonly=on,cache=unsafe: Must specify either driver or file

We need a further look, John. Sorry about not noticing this earlier, but in fact I should say thanks to your question!

Comment 6 Cole Robinson 2016-06-30 14:38:00 UTC
If qemu isn't going to change then the next best place for this is libvirt, it shouldn't put cache= on the command line if the device doesn't have any media. So reassigning to libvirt

Comment 8 Josh Stone 2016-06-30 17:02:56 UTC
But the media state of a cdrom drive is dynamic -- it may start empty, but what is supposed to happen with caching when media is inserted at runtime?  I'm not familiar with how libvirt actually communicates this to qemu, but is it able to specify the cache at insertion time?

Comment 11 Peter Krempa 2016-07-26 14:12:09 UTC
(In reply to John Snow from comment #4)
> As much as it bothers me as well, this will be CLOSED NOTABUG.
> 
> The cache=x option is a shorthand for three other cache options:
> 
> #define BDRV_OPT_CACHE_WB       "cache.writeback"
> #define BDRV_OPT_CACHE_DIRECT   "cache.direct"
> #define BDRV_OPT_CACHE_NO_FLUSH "cache.no-flush"
> 
> 
> The writeback option may be applied to an empty drive, but direct and
> no-flush may not be. By using the cache=x shorthand, you (inadvertently) set
> the other two options as well.
> 
> These options are then passed to blk_new_open, which rightly want a file to
> open.
> 
> Further, we are moving away from applying cache options to empty drives, so
> we will not be implementing the ability to apply cache options to empty
> drives (upstream).
> 
> The recommended workflow in the future is to use blockdev-add and specify
> cache options there (to open a new ISO) and then use
> x-blockdev-insert-medium to insert that disc into the drive.

Libvirt will eventually need to move to using these once blockdev-add is made more useful and x-blockdev-insert-medium loses the 'x-' prefix. Until the two commands are stable we can't really switch to use them though.

> We will continue to support (upstream) persistent cache options when using
> the legacy QMP/HMP commands "eject" and "change," but there are no plans to
> support persistent cache options on the CLI or through the newer QMP
> interface.

Unfortunately there's a problem. The 'change' command doesn't allow to set any cache mode.
Libvirt does not allow changing the cache mode during the lifetime but it depends on setting
the mode at start. This is fine with loaded cdroms, but breaks the use case of starting a VM with an empty cdrom drive.
 
> "OK, that's unfortunate, but the error message is very bad."

The more unfortunate part is the regression with existing configurations and the unstable interface that should be used instead.

Now for the constructive part:

Since libvirt always starts the VM with stopped cpus we could work the behavioral change around by using a dummy image and then ejecting it. This would trick qemu into remembering the cache options and not clearing them by using the legacy command. I didn't check though whether we'd need to do any capability checking in this case or any previous qemu supports this.

The other option would be to actually not introduce a regression in behaviour of course. (definitely a lot simpler on my part)

Comment 12 Peter Krempa 2016-08-02 10:59:56 UTC
After a brief discussion with others I think that having to do a workaround for a regression would be kind of gross and thus it's desired to fix this in qemu. Reassigning back to qemu.

Comment 13 Kevin Wolf 2016-09-14 08:57:42 UTC
(In reply to Peter Krempa from comment #11)
> Unfortunately there's a problem. The 'change' command doesn't allow to set
> any cache mode.  Libvirt does not allow changing the cache mode during the
> lifetime but it depends on setting the mode at start. This is fine with
> loaded cdroms, but breaks the use case of starting a VM with an empty cdrom
> drive.

The assumption that you're making here is totally reasonable, but unfortunately
it is wrong. The 'change' command has never kept the cache mode, but always
reset it to 'writethrough'. You may call it a bug, but it's been there from the
beginning.

From the commit message of commit 91a097e74:

    Note that this forbids specifying the cache mode for empty drives. It
    didn't make sense anyway to specify it there, because it didn't have any
    effect.

It seems that we actually do keep the cache mode across 'change' now, but that
is a change that was made later than the removal of the 'cache' option for
empty drives (didn't check it in detail, but commit 5433c24f the earliest). And
I'm not sure if it was intentional...

Of course, for read-only devices like CD-ROMs the cache mode doesn't make that
much of a difference anyway, so it shouldn't matter either way. We just need to
figure out whether libvirt stops setting the option, or whether qemu goes back
to accepting and silently ignoring it for empty drives.

Comment 14 Peter Krempa 2016-09-19 12:16:08 UTC
Fair enough. Looks like the downstream fix will work for now. We still will need to address that issue upstream though. I'll clone this BZ back to libvirt to track the progress of the real fix.

Comment 15 Miroslav Rezanina 2016-09-20 12:28:33 UTC
Fix included in qemu-kvm-rhev-2.6.0-26.el7

Comment 17 yduan 2016-09-23 06:32:17 UTC
Reproduced with qemu-kvm-rhev-2.6.0-3.el7.x86_64.

1. Start qemu with an empty cdrom, specify cache=none, qemu prompt errors:
# /usr/libexec/qemu-kvm \
 -device virtio-scsi-pci,id=scsi1 \
 -drive if=none,cache=none,media=cdrom,id=drive_cd,readonly=on \
 -device scsi-cd,bus=scsi1.0,drive=drive_cd,id=device_cd \

qemu-kvm: -drive if=none,cache=none,media=cdrom,id=drive_cd,readonly=on: Must specify either driver or file

2. Libvirt hit same issue
    <disk type='file' device='cdrom'>
      <driver name='qemu' type='raw' cache='none'/>
      <target dev='sdc' bus='scsi'/>
      <readonly/>
      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
    </disk>

# virsh start bug
error: Failed to start domain bug
error: internal error: qemu unexpectedly closed the monitor: 2016-09-23T06:28:58.555441Z qemu-kvm: -drive if=none,id=drive-scsi0-0-0-0,readonly=on,cache=none: Must specify either driver or file

***********************************************************************

Verified with qemu-kvm-rhev-2.6.0-26.el7.

There is no any error prompt when start qemu with an empty cdrom which with 'cache' option. So this issue has been fixed.

Comment 19 errata-xmlrpc 2016-11-07 21:14:29 UTC
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://rhn.redhat.com/errata/RHBA-2016-2673.html


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