[Qemu-devel] [PULL 0/7] Block patches

[Qemu-devel] [PULL 0/7] Block patches

[Kevin Wolf]

Some more fixes for 0.13.

The following changes since commit cdcf9153e5e17dde340135fee5dcc7c299f2d4f5:

  etrax: Update ethernet mgm-ctrl reg on writes (2010-07-25 21:03:56 +0200)

are available in the git repository at:
  git://repo.or.cz/qemu/kevin.git for-anthony

Alex Williamson (1):
      virtio-blk: Create exit function to unregister savevm

Aurelien Jarno (1):
      ide/atapi: add support for GET EVENT STATUS NOTIFICATION

Blue Swirl (1):
      Fix -snapshot deleting images on disk change

Bruce Rogers (1):
      move 'unsafe' to end of caching modes in help

Christoph Hellwig (1):
      block: default to 0 minimal / optiomal I/O size

Stefan Weil (1):
      block: Use error codes from lower levels for error message

Yoshiaki Tamura (1):
      block migration: propagate return value when bdrv_write() returns < 0

 block-migration.c |    6 +++++-
 block.c           |   32 ++++++++++++++++++++++++--------
 block.h           |    1 +
 block_int.h       |    4 ++--
 blockdev.c        |    1 +
 hw/ide/core.c     |   15 +++++++++++++++
 hw/virtio-blk.c   |    8 ++++++++
 hw/virtio-pci.c   |    1 +
 hw/virtio.h       |    1 +
 qemu-options.hx   |    2 +-
 10 files changed, 59 insertions(+), 12 deletions(-)

[Qemu-devel] [PATCH 1/7] ide/atapi: add support for GET EVENT STATUS NOTIFICATION

[Kevin Wolf]

From: Aurelien Jarno <aurelien@aurel32.net>

The GET EVENT STATUS NOTIFICATION is a mandatory command according
to MMC-3, even if event status notification is not supported.

This patch adds support for this command. It returns NEA ("No Event
Available") with an empty "Supported Event Classes" to show that it
doesn't event support status notification. If asychronous operation is
requested, which requires NCQ support, it returns an error according
to the specifications.

This fixes HAL support on FreeBSD and derivatives, which fill up the
logs every second with:

  acd0: FAILURE - unknown CMD (0x03) ILLEGAL REQUEST asc=0x20 ascq=0x00

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/core.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index e20f2e7..9e1bdd5 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1643,6 +1643,21 @@ static void ide_atapi_cmd(IDEState *s)
             ide_atapi_cmd_reply(s, len, max_len);
             break;
         }
+    case GPCMD_GET_EVENT_STATUS_NOTIFICATION:
+        max_len = ube16_to_cpu(packet + 7);
+
+        if (packet[1] & 0x01) { /* polling */
+            /* We don't support any event class (yet). */
+            cpu_to_ube16(buf, 0x00); /* No event descriptor returned */
+            buf[2] = 0x80;           /* No Event Available (NEA) */
+            buf[3] = 0x00;           /* Empty supported event classes */
+            ide_atapi_cmd_reply(s, 4, max_len);
+        } else { /* asynchronous mode */
+            /* Only polling is supported, asynchronous mode is not. */
+            ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
+                                ASC_INV_FIELD_IN_CMD_PACKET);
+        }
+        break;
     default:
         ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
                             ASC_ILLEGAL_OPCODE);
-- 
1.7.1.1

[Qemu-devel] [PATCH 2/7] block migration: propagate return value when bdrv_write() returns < 0

[Kevin Wolf]

From: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>

Currently block_load() doesn't check return value of bdrv_write(), and
even the destination weren't prepared to execute block migration, it
proceeds and guest boots on the target.  This patch fix this issue.

Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block-migration.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index a77106e..8eda307 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -586,6 +586,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
         addr >>= BDRV_SECTOR_BITS;
 
         if (flags & BLK_MIG_FLAG_DEVICE_BLOCK) {
+            int ret;
             /* get device name */
             len = qemu_get_byte(f);
             qemu_get_buffer(f, (uint8_t *)device_name, len);
@@ -601,9 +602,12 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
             buf = qemu_malloc(BLOCK_SIZE);
 
             qemu_get_buffer(f, buf, BLOCK_SIZE);
-            bdrv_write(bs, addr, buf, BDRV_SECTORS_PER_DIRTY_CHUNK);
+            ret = bdrv_write(bs, addr, buf, BDRV_SECTORS_PER_DIRTY_CHUNK);
 
             qemu_free(buf);
+            if (ret < 0) {
+                return ret;
+            }
         } else if (flags & BLK_MIG_FLAG_PROGRESS) {
             if (!banner_printed) {
                 printf("Receiving block device images\n");
-- 
1.7.1.1

[Qemu-devel] [PATCH 3/7] virtio-blk: Create exit function to unregister savevm

[Kevin Wolf]

From: Alex Williamson <alex.williamson@redhat.com>

Otherwise we can't migrate after we've removed a virtio block device.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/virtio-blk.c |    8 ++++++++
 hw/virtio-pci.c |    1 +
 hw/virtio.h     |    1 +
 3 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index f50069d..490cd41 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -28,6 +28,7 @@ typedef struct VirtIOBlock
     BlockConf *conf;
     unsigned short sector_mask;
     char sn[BLOCK_SERIAL_STRLEN];
+    DeviceState *qdev;
 } VirtIOBlock;
 
 static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
@@ -522,9 +523,16 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf)
     s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output);
 
     qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s);
+    s->qdev = dev;
     register_savevm(dev, "virtio-blk", virtio_blk_id++, 2,
                     virtio_blk_save, virtio_blk_load, s);
     bdrv_set_removable(s->bs, 0);
 
     return &s->vdev;
 }
+
+void virtio_blk_exit(VirtIODevice *vdev)
+{
+    VirtIOBlock *s = to_virtio_blk(vdev);
+    unregister_savevm(s->qdev, "virtio-blk", s);
+}
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 31a711e..17c3d15 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -569,6 +569,7 @@ static int virtio_blk_exit_pci(PCIDevice *pci_dev)
 {
     VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
 
+    virtio_blk_exit(proxy->vdev);
     blockdev_mark_auto_del(proxy->block.bs);
     return virtio_exit_pci(pci_dev);
 }
diff --git a/hw/virtio.h b/hw/virtio.h
index e4306cd..30e472a 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -194,6 +194,7 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf);
 
 
 void virtio_net_exit(VirtIODevice *vdev);
+void virtio_blk_exit(VirtIODevice *vdev);
 
 #define DEFINE_VIRTIO_COMMON_FEATURES(_state, _field) \
 	DEFINE_PROP_BIT("indirect_desc", _state, _field, \
-- 
1.7.1.1

[Qemu-devel] [PATCH 4/7] move 'unsafe' to end of caching modes in help

[Kevin Wolf]

From: Bruce Rogers <brogers@novell.com>

Libvirt parses qemu help output to determine qemu features. In particular
it probes for the following: "cache=writethrough|writeback|none". The
addition of the unsafe cache mode was inserted within this string, as
opposed to being added to the end, which impacted libvirt's probe.
Unbreak libvirt by keeping the existing cache modes intact and add
unsafe to the end.

This problem only manifests itself if a caching mode is explicitly
specified in the libvirt xml, in which case older syntax for caching is
passed to qemu, which it  no longer understands.

Signed-off-by: Bruce Rogers <brogers@novell.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-options.hx |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 0d7dd90..9ecc54e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -118,7 +118,7 @@ ETEXI
 DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
     "       [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
-    "       [,cache=writethrough|writeback|unsafe|none][,format=f]\n"
+    "       [,cache=writethrough|writeback|none|unsafe][,format=f]\n"
     "       [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
     "       [,readonly=on|off]\n"
     "                use 'file' as a drive image\n", QEMU_ARCH_ALL)
-- 
1.7.1.1

[Qemu-devel] Re: [PATCH 4/7] move 'unsafe' to end of caching modes in help

[Anthony Liguori]

[-- Attachment #1: Type: text/plain, Size: 1678 bytes --]

Hi Kevin,

There's an on-going discussion on this patch.  Could you drop it from your
pull request but leave it in your tree until the discussion is resolved?

Thanks,

Anthony Liguori

On Jul 26, 2010 9:01 AM, "Kevin Wolf" <kwolf@redhat.com> wrote:

From: Bruce Rogers <brogers@novell.com>

Libvirt parses qemu help output to determine qemu features. In particular
it probes for the following: "cache=writethrough|writeback|none". The
addition of the unsafe cache mode was inserted within this string, as
opposed to being added to the end, which impacted libvirt's probe.
Unbreak libvirt by keeping the existing cache modes intact and add
unsafe to the end.

This problem only manifests itself if a caching mode is explicitly
specified in the libvirt xml, in which case older syntax for caching is
passed to qemu, which it  no longer understands.

Signed-off-by: Bruce Rogers <brogers@novell.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-options.hx |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 0d7dd90..9ecc54e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -118,7 +118,7 @@ ETEXI
 DEF("drive", HAS_ARG, QEMU_OPTION_drive,
    "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
    "       [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
-    "       [,cache=writethrough|writeback|unsafe|none][,format=f]\n"
+    "       [,cache=writethrough|writeback|none|unsafe][,format=f]\n"
    "       [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
    "       [,readonly=on|off]\n"
    "                use 'file' as a drive image\n", QEMU_ARCH_ALL)
--
1.7.1.1

[-- Attachment #2: Type: text/html, Size: 2266 bytes --]

[Qemu-devel] Re: [PATCH 4/7] move 'unsafe' to end of caching modes in help

[Kevin Wolf]

Am 26.07.2010 19:28, schrieb Anthony Liguori:
> Hi Kevin,
> 
> There's an on-going discussion on this patch.  Could you drop it from
> your pull request but leave it in your tree until the discussion is
> resolved?

Right, so I guess waiting some hours was enough and you're going to pull
it without changes now?

Kevin

[Qemu-devel] Re: [PATCH 4/7] move 'unsafe' to end of caching modes in help

[Anthony Liguori]

On 07/27/2010 11:10 AM, Kevin Wolf wrote:
> Am 26.07.2010 19:28, schrieb Anthony Liguori:
>    
>> Hi Kevin,
>>
>> There's an on-going discussion on this patch.  Could you drop it from
>> your pull request but leave it in your tree until the discussion is
>> resolved?
>>      
> Right, so I guess waiting some hours was enough and you're going to pull
> it without changes now?
>    

Yeah, I'll do the pull now.  Sorry for the noise :-)

Regards,

Anthony Liguori

> Kevin
>    

[Qemu-devel] [PATCH 5/7] block: default to 0 minimal / optiomal I/O size

[Kevin Wolf]

From: Christoph Hellwig <hch@lst.de>

Currently we set them to 512 bytes unless manually specified.  Unforuntaly
some brain-dead partitioning tools create unaligned partitions if they
get low enough optiomal I/O size values, so don't report any at all
unless explicitly set.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block_int.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block_int.h b/block_int.h
index 96ff4cf..f075a8c 100644
--- a/block_int.h
+++ b/block_int.h
@@ -243,7 +243,7 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
                        _conf.logical_block_size, 512),                  \
     DEFINE_PROP_UINT16("physical_block_size", _state,                   \
                        _conf.physical_block_size, 512),                 \
-    DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 512),  \
-    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 512)
+    DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),  \
+    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0)
 
 #endif /* BLOCK_INT_H */
-- 
1.7.1.1

[Qemu-devel] [PATCH 6/7] block: Use error codes from lower levels for error message

[Kevin Wolf]

From: Stefan Weil <weil@mail.berlios.de>

"No such file or directory" is a misleading error message
when a user tries to open a file with wrong permissions.

Cc: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Weil <weil@mail.berlios.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c |   27 +++++++++++++++++++--------
 1 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index f837876..49e6cbc 100644
--- a/block.c
+++ b/block.c
@@ -330,7 +330,7 @@ BlockDriver *bdrv_find_protocol(const char *filename)
     return NULL;
 }
 
-static BlockDriver *find_image_format(const char *filename)
+static int find_image_format(const char *filename, BlockDriver **pdrv)
 {
     int ret, score, score_max;
     BlockDriver *drv1, *drv;
@@ -338,19 +338,27 @@ static BlockDriver *find_image_format(const char *filename)
     BlockDriverState *bs;
 
     ret = bdrv_file_open(&bs, filename, 0);
-    if (ret < 0)
-        return NULL;
+    if (ret < 0) {
+        *pdrv = NULL;
+        return ret;
+    }
 
     /* Return the raw BlockDriver * to scsi-generic devices or empty drives */
     if (bs->sg || !bdrv_is_inserted(bs)) {
         bdrv_delete(bs);
-        return bdrv_find_format("raw");
+        drv = bdrv_find_format("raw");
+        if (!drv) {
+            ret = -ENOENT;
+        }
+        *pdrv = drv;
+        return ret;
     }
 
     ret = bdrv_pread(bs, 0, buf, sizeof(buf));
     bdrv_delete(bs);
     if (ret < 0) {
-        return NULL;
+        *pdrv = NULL;
+        return ret;
     }
 
     score_max = 0;
@@ -364,7 +372,11 @@ static BlockDriver *find_image_format(const char *filename)
             }
         }
     }
-    return drv;
+    if (!drv) {
+        ret = -ENOENT;
+    }
+    *pdrv = drv;
+    return ret;
 }
 
 /**
@@ -571,12 +583,11 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
 
     /* Find the right image format driver */
     if (!drv) {
-        drv = find_image_format(filename);
+        ret = find_image_format(filename, &drv);
         probed = 1;
     }
 
     if (!drv) {
-        ret = -ENOENT;
         goto unlink_and_fail;
     }
 
-- 
1.7.1.1

[Qemu-devel] [PATCH 7/7] Fix -snapshot deleting images on disk change

[Kevin Wolf]

From: Blue Swirl <blauwirbel@gmail.com>

Block device change command did not copy BDRV_O_SNAPSHOT flag. Thus
the new image did not have this flag and the file got deleted during
opening.

Fix by copying BDRV_O_SNAPSHOT flag.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c    |    5 +++++
 block.h    |    1 +
 blockdev.c |    1 +
 3 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 49e6cbc..452ae94 100644
--- a/block.c
+++ b/block.c
@@ -1811,6 +1811,11 @@ int bdrv_can_snapshot(BlockDriverState *bs)
     return 1;
 }
 
+int bdrv_is_snapshot(BlockDriverState *bs)
+{
+    return !!(bs->open_flags & BDRV_O_SNAPSHOT);
+}
+
 BlockDriverState *bdrv_snapshots(void)
 {
     BlockDriverState *bs;
diff --git a/block.h b/block.h
index c2a7e4c..db131a3 100644
--- a/block.h
+++ b/block.h
@@ -202,6 +202,7 @@ const char *bdrv_get_encrypted_filename(BlockDriverState *bs);
 void bdrv_get_backing_filename(BlockDriverState *bs,
                                char *filename, int filename_size);
 int bdrv_can_snapshot(BlockDriverState *bs);
+int bdrv_is_snapshot(BlockDriverState *bs);
 BlockDriverState *bdrv_snapshots(void);
 int bdrv_snapshot_create(BlockDriverState *bs,
                          QEMUSnapshotInfo *sn_info);
diff --git a/blockdev.c b/blockdev.c
index 0a9dec3..01e402b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -590,6 +590,7 @@ int do_change_block(Monitor *mon, const char *device,
         return -1;
     }
     bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
+    bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
     if (bdrv_open(bs, filename, bdrv_flags, drv) < 0) {
         qerror_report(QERR_OPEN_FILE_FAILED, filename);
         return -1;
-- 
1.7.1.1