Bug 1402645 - Required cache.direct=on when set aio=native
Summary: Required cache.direct=on when set aio=native
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: qemu-kvm-rhev   
(Show other bugs)
Version: 7.4
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: rc
: ---
Assignee: John Snow
QA Contact: CongLi
URL:
Whiteboard:
Keywords: Regression
: 1425002 (view as bug list)
Depends On:
Blocks: 1406973
TreeView+ depends on / blocked
 
Reported: 2016-12-08 02:54 UTC by jingzhao
Modified: 2017-08-02 03:35 UTC (History)
19 users (show)

Fixed In Version: qemu-kvm-rhev-2.9.0-5.el7
Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2017-08-01 23:39:45 UTC
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)


External Trackers
Tracker ID Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2017:2392 normal SHIPPED_LIVE Important: qemu-kvm-rhev security, bug fix, and enhancement update 2017-08-01 20:04:36 UTC

Description jingzhao 2016-12-08 02:54:57 UTC
Description of problem:
aio=native was specified, but it requires cache.direct=on, which was not specified.


Version-Release number of selected component (if applicable):
qemu-kvm-rhev-2.6.0-29.el7.x86_64

How reproducible:
3/3

Steps to Reproduce:
1.Boot guest with following cli
/usr/libexec/qemu-kvm -drive id=drive_none,if=none,snapshot=off,aio=native,cache=none,media=cdrom -device ide-cd,id=none,drive=drive_none,bootindex=1,bus=ide.0,unit=0 -monitor stdio

2.(qemu) change drive_none test.iso
aio=native was specified, but it requires cache.direct=on, which was not specified.



Actual results:
aio=native was specified, but it requires cache.direct=on, which was not specified.


Expected results:
No error and change cdrom correctly

Additional info:
1. Tested it with qemu-kvm-rhev.x86_64 10:2.3.0-31.el7_2.21 and didn't hit the issue, following are the detailed 
QEMU 2.3.0 monitor - type 'help' for more information
(qemu) VNC server running on `127.0.0.1:5900'

(qemu) info block
drive_none: [not inserted]
    Removable device: not locked, tray closed

floppy0: [not inserted]
    Removable device: not locked, tray closed

sd0: [not inserted]
    Removable device: not locked, tray closed
(qemu) change drive_none /home/test.iso
(qemu) info block
drive_none: /home/test.iso (raw, read-only)
    Removable device: not locked, tray closed
    Cache mode:       writethrough

floppy0: [not inserted]
    Removable device: not locked, tray closed

sd0: [not inserted]
    Removable device: not locked, tray closed


2. Tested it with qemu-kvm-1.5.3-126.el7.x86_64.rpm and didn't hit the issue

Comment 1 jingzhao 2016-12-08 02:59:12 UTC
Also can reproduce the issue on qemu-kvm-rhev-2.6.0-28.el7_3.1.x86_64

Comment 5 Kevin Wolf 2016-12-08 15:00:33 UTC
This is essentially related to the same thing as bug 1342999.

Upstream will already reject setting a cache mode for an empty CD-ROM drive
because it has no effect. For compatibility reasons, we made RHEL still accept
and silently ignore the option. It doesn't make any sense to specify it, but
libvirt happened to do so.

We could probably extend this downstream-only hack and also ignore aio=native
silently for empty CD-ROM drives, but unless libvirt is making use of it, is it
really worth more downstream-only changes?

On the other hand, the fix could be as easy as this:

--- a/blockdev.c
+++ b/blockdev.c
@@ -603,7 +603,7 @@ 
         }
 
         blk_rs = blk_get_root_state(blk);
-        blk_rs->open_flags    = bdrv_flags;
+        blk_rs->open_flags    = bdrv_flags & ~BDRV_O_NATIVE_AIO;
         blk_rs->read_only     = !(bdrv_flags & BDRV_O_RDWR);
         blk_rs->detect_zeroes = detect_zeroes;


Anyway, reassigning to John who owned bug 1342999.

Comment 6 Ademar Reis 2016-12-27 14:43:57 UTC
(In reply to Kevin Wolf from comment #5)
> This is essentially related to the same thing as bug 1342999.
> 
> Upstream will already reject setting a cache mode for an empty CD-ROM drive
> because it has no effect. For compatibility reasons, we made RHEL still
> accept
> and silently ignore the option. It doesn't make any sense to specify it, but
> libvirt happened to do so.
> 
> We could probably extend this downstream-only hack and also ignore aio=native
> silently for empty CD-ROM drives, but unless libvirt is making use of it, is
> it really worth more downstream-only changes?

Since we don't support direct usage of QEMU in RHEL-7, there's no need for the downstream-only change if libvirt is not using it.

Comment 7 John Snow 2017-02-28 01:07:05 UTC
Kevin, shouldn't we be *actively* rejecting the aio property on empty drives upstream? Especially if we can set it via blockdev_add, and the "Cache modes on removable media devices is already broken" genie is already out of the bottle.

I am not sure the exact right way to factor this out of the existing spaghetti, though. I'd suppose that the aio option needs to stay in bs_opts and not get parsed out into opts and subsequently bdrv_flags via extract_common_blockdev_options...

However, that does mean that:

(1) On initial insert, we'd be prohibiting an AIO mode for empty drives, but
(2) On subsequent changes, though, if a user added an AIO mode to an inserted medium, it would persist through the root state.

Which is also kind of madness, isn't it?

Just simply ignoring it downstream doesn't seem right as this exact same kind of behavior will still exist upstream, and it seems just as wrong there.

Looking for suggestions on what you feel the right way to untangle this knot is.

Comment 8 Kevin Wolf 2017-03-16 19:02:02 UTC
As discussed on IRC a while ago, yes, we should, and yes, users will complain if
we do it. We have messed up AIO by making it a BDRV_O_* flag when it is really
specific to file-posix and file-win32, and I'm not sure how to fix it without
breaking compatibility. The "correct" thing would be to remove it from the root
state, but I'm sure that people expect that it's kept across media change.

(I know, not that useful, I just need to get rid of the needinfo...)

Comment 9 Eric Blake 2017-05-11 18:54:16 UTC
(In reply to Kevin Wolf from comment #5)
> This is essentially related to the same thing as bug 1342999.
> 
> Upstream will already reject setting a cache mode for an empty CD-ROM drive
> because it has no effect. For compatibility reasons, we made RHEL still
> accept
> and silently ignore the option. It doesn't make any sense to specify it, but
> libvirt happened to do so.
> 
> We could probably extend this downstream-only hack and also ignore aio=native
> silently for empty CD-ROM drives, but unless libvirt is making use of it, is
> it
> really worth more downstream-only changes?
> 
> On the other hand, the fix could be as easy as this:
> 
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -603,7 +603,7 @@ 
>          }
>  
>          blk_rs = blk_get_root_state(blk);
> -        blk_rs->open_flags    = bdrv_flags;
> +        blk_rs->open_flags    = bdrv_flags & ~BDRV_O_NATIVE_AIO;
>          blk_rs->read_only     = !(bdrv_flags & BDRV_O_RDWR);
>          blk_rs->detect_zeroes = detect_zeroes;
> 
> 
> Anyway, reassigning to John who owned bug 1342999.

bug 1425003 comment 3 includes a formula where libvirt XML can indeed trigger this

Comment 10 Eric Blake 2017-05-12 18:19:15 UTC
*** Bug 1425002 has been marked as a duplicate of this bug. ***

Comment 11 Miroslav Rezanina 2017-05-16 13:02:59 UTC
Fix included in qemu-kvm-rhev-2.9.0-5.el7

Comment 13 CongLi 2017-05-23 01:10:42 UTC
Reproduced this bug on qemu-kvm-rhev-2.8.0-6.el7.x86_64

Steps:

1. Boot guest with following cli
/usr/libexec/qemu-kvm -drive id=drive_none,if=none,snapshot=off,aio=native,cache=none,media=cdrom -device ide-cd,id=none,drive=drive_none,bootindex=1,bus=ide.0,unit=0 -monitor stdio

2. Change cdrom
(qemu) change drive_none /root/test.iso 
aio=native was specified, but it requires cache.direct=on, which was not specified.
(qemu) info block
drive_none: [not inserted]
    Removable device: not locked, tray closed

floppy0: [not inserted]
    Removable device: not locked, tray closed

sd0: [not inserted]
    Removable device: not locked, tray closed


Verified this bug on qemu-kvm-rhev-2.9.0-5.el7.x86_64

(qemu) change drive_none /root/test.iso 
(qemu) info block
drive_none (#block144): /root/test.iso (raw, read-only)
    Removable device: not locked, tray closed
    Cache mode:       writeback

floppy0: [not inserted]
    Removable device: not locked, tray closed

sd0: [not inserted]
    Removable device: not locked, tray closed
 

Thanks.

Comment 15 errata-xmlrpc 2017-08-01 23:39:45 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://access.redhat.com/errata/RHSA-2017:2392

Comment 16 errata-xmlrpc 2017-08-02 01:17:23 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://access.redhat.com/errata/RHSA-2017:2392

Comment 17 errata-xmlrpc 2017-08-02 02:09:23 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://access.redhat.com/errata/RHSA-2017:2392

Comment 18 errata-xmlrpc 2017-08-02 02:50:09 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://access.redhat.com/errata/RHSA-2017:2392

Comment 19 errata-xmlrpc 2017-08-02 03:14:52 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://access.redhat.com/errata/RHSA-2017:2392

Comment 20 errata-xmlrpc 2017-08-02 03:35:00 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://access.redhat.com/errata/RHSA-2017:2392


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