All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: anthony@codemonkey.ws
Cc: kwolf@redhat.com, qemu-devel@nongnu.org
Subject: [Qemu-devel] [PATCH 04/14] monitor: allow device to be ejected if no disk is inserted
Date: Tue, 22 Jun 2010 16:09:23 +0200	[thread overview]
Message-ID: <1277215773-27357-5-git-send-email-kwolf@redhat.com> (raw)
In-Reply-To: <1277215773-27357-1-git-send-email-kwolf@redhat.com>

From: Eduardo Habkost <ehabkost@redhat.com>

This changes the monitor eject_device() function to not check for
bdrv_is_inserted().

Example run where the bug manifests itself:

(output of 'info block' is stripped to include only the CD-ROM device)

  (qemu) info block
  ide1-cd0: type=cdrom removable=1 locked=0 [not inserted]
  (qemu) change ide1-cd0 /dev/cdrom host_cdrom
  (qemu) info block
  ide1-cd0: type=cdrom removable=1 locked=0 file=/dev/cdrom ro=1 drv=host_cdrom encrypted=0
  (qemu) eject ide1-cd0
  (qemu) info block
  ide1-cd0: type=cdrom removable=1 locked=0 file=/dev/cdrom ro=1 drv=host_cdrom encrypted=0

  # at this point, a disk was inserted on the host CD-ROM drive

  (qemu) info block
  ide1-cd0: type=cdrom removable=1 locked=0 file=/dev/cdrom ro=1 drv=host_cdrom encrypted=0
  (qemu) eject ide1-cd0
  (qemu) info block
  ide1-cd0: type=cdrom removable=1 locked=0 [not inserted]
  (qemu)

The first eject command didn't work because the is_inserted() check
failed.

I have no clue why the code had the is_inserted() check, as it doesn't matter
if there is a disk present at the host drive, when the user wants the virtual
device to be disconnected from the host device.

The is_inserted() check has another side effect: a memory leak if the "change"
command is used multiple times, as do_change() calls eject_device() before
re-opening the block device, but bdrv_close() is never called.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c |   22 ++++++++++------------
 1 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index b376884..3b8c606 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -503,20 +503,18 @@ void do_commit(Monitor *mon, const QDict *qdict)
 
 static int eject_device(Monitor *mon, BlockDriverState *bs, int force)
 {
-    if (bdrv_is_inserted(bs)) {
-        if (!force) {
-            if (!bdrv_is_removable(bs)) {
-                qerror_report(QERR_DEVICE_NOT_REMOVABLE,
-                               bdrv_get_device_name(bs));
-                return -1;
-            }
-            if (bdrv_is_locked(bs)) {
-                qerror_report(QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
-                return -1;
-            }
+    if (!force) {
+        if (!bdrv_is_removable(bs)) {
+            qerror_report(QERR_DEVICE_NOT_REMOVABLE,
+                           bdrv_get_device_name(bs));
+            return -1;
+        }
+        if (bdrv_is_locked(bs)) {
+            qerror_report(QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
+            return -1;
         }
-        bdrv_close(bs);
     }
+    bdrv_close(bs);
     return 0;
 }
 
-- 
1.6.6.1

  parent reply	other threads:[~2010-06-22 14:09 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-22 14:09 [Qemu-devel] [PULL 00/14] Block patches Kevin Wolf
2010-06-22 14:09 ` [Qemu-devel] [PATCH 01/14] scsi-bus: Add PERSISTENT_RESERVE_OUT SCSIRequest->cmd.mode setup Kevin Wolf
2010-06-22 14:09 ` [Qemu-devel] [PATCH 02/14] scsi-bus: Add MAINTENANCE_IN and MAINTENANCE_OUT SCSIRequest xfer and mode assignments Kevin Wolf
2010-06-22 14:09 ` [Qemu-devel] [PATCH 03/14] block: fix physical_block_size calculation Kevin Wolf
2010-06-22 14:09 ` Kevin Wolf [this message]
2010-06-22 14:09 ` [Qemu-devel] [PATCH 05/14] block: Add bdrv_(p)write_sync Kevin Wolf
2010-06-22 14:09 ` [Qemu-devel] [PATCH 06/14] cow: Use bdrv_(p)write_sync for metadata writes Kevin Wolf
2010-06-22 14:09 ` [Qemu-devel] [PATCH 07/14] qcow: " Kevin Wolf
2010-06-22 14:09 ` [Qemu-devel] [PATCH 08/14] qcow2: " Kevin Wolf
2010-06-22 14:09 ` [Qemu-devel] [PATCH 09/14] vmdk: " Kevin Wolf
2010-06-22 14:09 ` [Qemu-devel] [PATCH 10/14] vpc: " Kevin Wolf
2010-06-22 14:09 ` [Qemu-devel] [PATCH 11/14] qemu-io: check registered fds in command_loop() Kevin Wolf
2010-06-22 14:09 ` [Qemu-devel] [PATCH 12/14] virtio-blk: fix the list operation in virtio_blk_load() Kevin Wolf
2010-06-22 14:09 ` [Qemu-devel] [PATCH 13/14] qcow2: Fix qemu-img check segfault on corrupted images Kevin Wolf
2010-06-22 14:09 ` [Qemu-devel] [PATCH 14/14] qcow2: Don't try to check tables that couldn't be loaded Kevin Wolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1277215773-27357-5-git-send-email-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.