qemu-nbd -r -c taints device for subsequent usage, even after -d

[Robert Collins]

Something about qemu-nbd -r -c /dev/nbd0 someimg leaves cruft behind - subsequent connections get marked readonly.

This is on quantal, haven't checked precise or raring.

To demonstrate:
# use one image
qemu-img create -f qcow2 /tmp/1.qcow2 100M
sudo qemu-nbd -c /dev/nbd2 /tmp/1.qcow2
sudo mkfs -t ext4 /dev/nbd2
sudo qemu-nbd -d /dev/nbd2
# use a second one on the same nbd device, shows that reuse works:
qemu-img create -f qcow2 /tmp/2.qcow2 100M
sudo qemu-nbd -c /dev/nbd2 /tmp/2.qcow2
sudo mkfs -t ext4 /dev/nbd2
sudo qemu-nbd -d /dev/nbd2
# connect an image in read only mode
sudo qemu-nbd -r -c /dev/nbd2 /tmp/2.qcow2
sudo dumpe2fs /dev/nbd2 | head -n 3
sudo qemu-nbd -d /dev/nbd2
# now try to reuse in read-write mode again:
qemu-img create -f qcow2 /tmp/3.qcow2 100M
sudo qemu-nbd -c /dev/nbd2 /tmp/3.qcow2
sudo mkfs -t ext4 /dev/nbd2
# here it goes boom:
mke2fs 1.42.5 (29-Jul-2012)
/dev/nbd2: Operation not permitted while setting up superblock
# still need to cleanup
sudo qemu-nbd -d /dev/nbd2

===============================
SRU Justification:
1. Impact: mounting an nbd device as read-write after doing so read-only
will cause the mount to erroneously (and quietly) be read-only.
2. Development fix: have qemu-nbd set the device to read-write when asked,
rather than only setting read-only.
3. Stable fix: same as development fix
4. Test case: See above
5. Regression potential: The patch is localized to the handling of read-only
flag in qemu-nbd, so any regression should not affect anything else.
===============================

[Soren Hansen (soren)]

Happens on Precise as well.

[Robert Collins (lifeless)]

Quick code read - I think that this block:
    if (flags & NBD_FLAG_READ_ONLY) {
        int read_only = 1;
        TRACE("Setting readonly attribute");

        if (ioctl(fd, BLKROSET, (unsigned long) &read_only) < 0) {
            int serrno = errno;
            LOG("Failed setting read-only attribute");
            return -serrno;
        }
    }

in nbd.c should be
    {
        int read_only = 0;
        if (flags & NBD_FLAG_READ_ONLY)
            read_only = 1;
        TRACE("Setting readonly attribute");
        if (ioctl(fd, BLKROSET, (unsigned long) &read_only) < 0) {
            int serrno = errno;
            LOG("Failed setting read-only attribute");
            return -serrno;
        }
    }

[Robert Collins (lifeless)]

http://paste.ubuntu.com/1352684/ is a debdiff, uploading the source format 3 patch as well

[Robert Collins (lifeless)]

Fixed patch - I had my sense inverted... http://paste.ubuntu.com/1352711/

[Serge Hallyn (serge-hallyn)]

Thanks, this still applies upstream as well.

[Paolo Bonzini (bonzini)]

To some extent it is a bug in the upstream kernel, which doesn't reset state properly. However, the qemu patch is also good. Thanks!

[Serge Hallyn (serge-hallyn)]

Thanks, Paul, I'll cherrypick commit c8969eded252058e90e91f12f75f32aceae46ec9 into the ubuntu packages

[Launchpad Janitor (janitor)]

This bug was fixed in the package qemu-kvm - 1.2.0+noroms-0ubuntu4

---------------
qemu-kvm (1.2.0+noroms-0ubuntu4) raring; urgency=low

  [ Serge Hallyn ]
  * debian/qemu-kvm.postinst: use udevadm trigger to change /dev/kvm perms as
    recommended by Steve Langasek (LP: #1057024)
  * apply debian/patches/nbd-fixes-to-read-only-handling.patch from upstream to
    make read-write mount after read-only mount work. (LP: #1077838)

  [ Robert Collins ]
  * Fix upstart job to succeed if ksm settings can't be altered in the same way
    other settings are handled. (LP: #1078530)
 -- Serge Hallyn <email address hidden> Wed, 14 Nov 2012 11:30:14 -0600

[Adam Conrad (adconrad)]

Hello Robert, or anyone else affected,

Accepted qemu-kvm into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/qemu-kvm/1.0+noroms-0ubuntu14.4 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please change the bug tag from verification-needed to verification-done. If it does not, change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Adam Conrad (adconrad)]

Hello Robert, or anyone else affected,

Accepted qemu-kvm into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/qemu-kvm/1.0+noroms-0ubuntu14.5 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please change the bug tag from verification-needed to verification-done. If it does not, change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Adam Conrad (adconrad)]

Hello Robert, or anyone else affected,

Accepted qemu-kvm into quantal-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/qemu-kvm/1.2.0+noroms-0ubuntu2.12.10.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please change the bug tag from verification-needed to verification-done. If it does not, change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Serge Hallyn (serge-hallyn)]

Verified on precise.

[Serge Hallyn (serge-hallyn)]

Verified on quantal.

[Adam Conrad (adconrad)]

Hello Robert, or anyone else affected,

Accepted qemu-kvm into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/qemu-kvm/1.0+noroms-0ubuntu14.6 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Serge Hallyn (serge-hallyn)]

Re-verified in precise.

[Scott Kitterman (kitterman)]

The verification of this Stable Release Update has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regresssions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package qemu-kvm - 1.2.0+noroms-0ubuntu2.12.10.1

---------------
qemu-kvm (1.2.0+noroms-0ubuntu2.12.10.1) quantal-proposed; urgency=low

  [ Serge Hallyn ]
  * debian/qemu-kvm.postinst: use udevadm trigger to change /dev/kvm perms as
    recommended by Steve Langasek (LP: #1057024)
  * apply debian/patches/nbd-fixes-to-read-only-handling.patch from upstream to
    make read-write mount after read-only mount work. (LP: #1077838)
  * make qemu-kvm depend on udev (LP: #1080912)

  [ Robert Collins ]
  * Fix upstart job to succeed if ksm settings can't be altered in the same way
    other settings are handled. (LP: #1078530)
 -- Serge Hallyn <email address hidden> Mon, 19 Nov 2012 09:15:42 -0600

[Launchpad Janitor (janitor)]

This bug was fixed in the package qemu-kvm - 1.0+noroms-0ubuntu14.6

---------------
qemu-kvm (1.0+noroms-0ubuntu14.6) precise-proposed; urgency=low

  * Fix qemu-kvm.upstart: just don't run in a container. Otherwise we'll
    still try to load/unload kernel modules. Also undo the || true after
    sysfs writes. Since setting those is a part of configuring qemu-kvm
    on the host, failing when they fail makes sense.

qemu-kvm (1.0+noroms-0ubuntu14.5) precise-proposed; urgency=low

  * add udev to qemu-kvm Depends to ensure that postinst succeeds.
    (LP: #1080912)

qemu-kvm (1.0+noroms-0ubuntu14.4) precise-proposed; urgency=low

  [ Serge Hallyn ]
  * debian/qemu-kvm.postinst: use udevadm trigger to change /dev/kvm perms as
    recommended by Steve Langasek (LP: #1057024)
  * apply debian/patches/nbd-fixes-to-read-only-handling.patch from upstream to
    make read-write mount after read-only mount work. (LP: #1077838)

  [ Robert Collins ]
  * Fix upstart job to succeed if ksm settings can't be altered in the same way
    other settings are handled. (LP: #1078530)
 -- Serge Hallyn <email address hidden> Thu, 20 Dec 2012 12:34:52 -0600

[Thomas Huth (th-huth)]

According to comment #9 this bug has been fixed by this commit here:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=c8969eded252058
... so I think it should be OK to close this bug now.