[Qemu-devel] [PULL 00/11] Block patches

[Qemu-devel] [PULL 00/11] Block patches

[Kevin Wolf]

The following changes since commit e0a1e32dbc41e6b2aabb436a9417dfd32177a3dc:

  Merge branch 'usb.64' of git://git.kraxel.org/qemu (2012-09-11 18:06:56 +0200)

are available in the git repository at:

  git://repo.or.cz/qemu/kevin.git for-anthony

Benoît Canet (2):
      qapi: Add SnapshotInfo and ImageInfo.
      qemu-img: Add json output option to the info command.

Daniel P. Berrange (1):
      Don't require encryption password for 'qemu-img info' command

Dunrong Huang (1):
      block: Don't forget to delete temporary file

Jason Baron (1):
      ahci: properly reset PxCMD on HBA reset

MORITA Kazutaka (1):
      sheepdog: fix savevm and loadvm

Pavel Hrdina (1):
      block: fix block tray status

Ronnie Sahlberg (1):
      ATAPI: STARTSTOPUNIT only eject/load media if powercondition is 0

Stefan Weil (3):
      ide: Fix error messages from static code analysis (no real error)
      block/curl: Fix wrong free statement
      vdi: Fix warning from clang

 Makefile         |    3 +-
 block.c          |   10 ++-
 block/curl.c     |    3 +-
 block/sheepdog.c |    3 +-
 block/vdi.c      |   25 +++---
 hw/ide/ahci.c    |    2 +-
 hw/ide/atapi.c   |    6 ++
 hw/ide/core.c    |   11 +--
 qapi-schema.json |   64 ++++++++++++++
 qemu-img-cmds.hx |    4 +-
 qemu-img.c       |  257 ++++++++++++++++++++++++++++++++++++++++++-----------
 qemu-img.texi    |    5 +-
 12 files changed, 305 insertions(+), 88 deletions(-)

[Qemu-devel] [PATCH 01/11] sheepdog: fix savevm and loadvm

[Kevin Wolf]

From: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>

This patch sets data to be sent to Sheepdog correctly and fixes savevm
and loadvm operations on a Sheepdog image.

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/sheepdog.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index df4f441..e0753ee 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1986,7 +1986,7 @@ static int do_load_save_vmstate(BDRVSheepdogState *s, uint8_t *data,
         vdi_index = pos / SD_DATA_OBJ_SIZE;
         offset = pos % SD_DATA_OBJ_SIZE;
 
-        data_len = MIN(remaining, SD_DATA_OBJ_SIZE);
+        data_len = MIN(remaining, SD_DATA_OBJ_SIZE - offset);
 
         vmstate_oid = vid_to_vmstate_oid(s->inode.vdi_id, vdi_index);
 
@@ -2007,6 +2007,7 @@ static int do_load_save_vmstate(BDRVSheepdogState *s, uint8_t *data,
         }
 
         pos += data_len;
+        data += data_len;
         remaining -= data_len;
     }
     ret = size;
-- 
1.7.6.5

[Qemu-devel] [PATCH 02/11] ATAPI: STARTSTOPUNIT only eject/load media if powercondition is 0

[Kevin Wolf]

From: Ronnie Sahlberg <ronniesahlberg@gmail.com>

The START STOP UNIT command will only eject/load media if
power condition is zero.

If power condition is !0 then LOEJ and START will be ignored.

>From MMC (sbc contains similar wordings too)
  The Power Conditions field requests the block device to be placed
  in the power condition defined in
  Table 558. If this field has a value other than 0h then the Start
  and LoEj bits shall be ignored.

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/atapi.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index f7f714c..685cbaa 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -875,6 +875,12 @@ static void cmd_start_stop_unit(IDEState *s, uint8_t* buf)
     int sense;
     bool start = buf[4] & 1;
     bool loej = buf[4] & 2;     /* load on start, eject on !start */
+    int pwrcnd = buf[4] & 0xf0;
+
+    if (pwrcnd) {
+        /* eject/load only happens for power condition == 0 */
+        return;
+    }
 
     if (loej) {
         if (!start && !s->tray_open && s->tray_locked) {
-- 
1.7.6.5

[Qemu-devel] [PATCH 03/11] ide: Fix error messages from static code analysis (no real error)

[Kevin Wolf]

From: Stefan Weil <sw@weilnetz.de>

Report from smatch:
hw/ide/core.c:1472 ide_exec_cmd(423) error: buffer overflow 'smart_attributes' 8 <= 29
hw/ide/core.c:1474 ide_exec_cmd(425) error: buffer overflow 'smart_attributes' 8 <= 29
hw/ide/core.c:1475 ide_exec_cmd(426) error: buffer overflow 'smart_attributes' 8 <= 29
...

The upper limit of 30 was never reached because both for loops terminated
when 'smart_attributes' reached end of list, so there was no real buffer
overflow.

Nevertheless, changing the code not only fixes the error report, but also
reduces the size of smart_attributes and simplifies the for loops.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/core.c |   11 ++---------
 1 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index d65ef3d..d6fb69c 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -53,8 +53,6 @@ static const int smart_attributes[][12] = {
     { 0x0c, 0x03, 0x00, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
     /* airflow-temperature-celsius */
     { 190,  0x03, 0x00, 0x45, 0x45, 0x1f, 0x00, 0x1f, 0x1f, 0x00, 0x00, 0x32},
-    /* end of list */
-    { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
 };
 
 static int ide_handle_rw_error(IDEState *s, int error, int op);
@@ -1468,9 +1466,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
 	case SMART_READ_THRESH:
 		memset(s->io_buffer, 0, 0x200);
 		s->io_buffer[0] = 0x01; /* smart struct version */
-		for (n=0; n<30; n++) {
-		if (smart_attributes[n][0] == 0)
-			break;
+		for (n = 0; n < ARRAY_SIZE(smart_attributes); n++) {
 		s->io_buffer[2+0+(n*12)] = smart_attributes[n][0];
 		s->io_buffer[2+1+(n*12)] = smart_attributes[n][11];
 		}
@@ -1484,10 +1480,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
 	case SMART_READ_DATA:
 		memset(s->io_buffer, 0, 0x200);
 		s->io_buffer[0] = 0x01; /* smart struct version */
-		for (n=0; n<30; n++) {
-		    if (smart_attributes[n][0] == 0) {
-			break;
-		    }
+		for (n = 0; n < ARRAY_SIZE(smart_attributes); n++) {
 		    int i;
 		    for(i = 0; i < 11; i++) {
 			s->io_buffer[2+i+(n*12)] = smart_attributes[n][i];
-- 
1.7.6.5

[Qemu-devel] [PATCH 04/11] block/curl: Fix wrong free statement

[Kevin Wolf]

From: Stefan Weil <sw@weilnetz.de>

Report from smatch:
block/curl.c:546 curl_close(21) info: redundant null check on s->url calling free()

The check was redundant, and free was also wrong because the memory
was allocated using g_strdup.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/curl.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index e7c3634..c1074cd 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -542,8 +542,7 @@ static void curl_close(BlockDriverState *bs)
     }
     if (s->multi)
         curl_multi_cleanup(s->multi);
-    if (s->url)
-        free(s->url);
+    g_free(s->url);
 }
 
 static int64_t curl_getlength(BlockDriverState *bs)
-- 
1.7.6.5

[Qemu-devel] [PATCH 05/11] vdi: Fix warning from clang

[Kevin Wolf]

From: Stefan Weil <sw@weilnetz.de>

ccc-analyzer reports these warnings:

block/vdi.c:704:13: warning: Dereference of null pointer
            bmap[i] = VDI_UNALLOCATED;
            ^
block/vdi.c:702:13: warning: Dereference of null pointer
            bmap[i] = i;
            ^

Moving some code into the if block fixes this.
It also avoids calling function write with 0 bytes of data.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vdi.c |   25 ++++++++++++-------------
 1 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/block/vdi.c b/block/vdi.c
index c4f1529..550cf58 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -628,7 +628,6 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options)
     VdiHeader header;
     size_t i;
     size_t bmap_size;
-    uint32_t *bmap;
 
     logout("\n");
 
@@ -693,21 +692,21 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options)
         result = -errno;
     }
 
-    bmap = NULL;
     if (bmap_size > 0) {
-        bmap = (uint32_t *)g_malloc0(bmap_size);
-    }
-    for (i = 0; i < blocks; i++) {
-        if (image_type == VDI_TYPE_STATIC) {
-            bmap[i] = i;
-        } else {
-            bmap[i] = VDI_UNALLOCATED;
+        uint32_t *bmap = g_malloc0(bmap_size);
+        for (i = 0; i < blocks; i++) {
+            if (image_type == VDI_TYPE_STATIC) {
+                bmap[i] = i;
+            } else {
+                bmap[i] = VDI_UNALLOCATED;
+            }
         }
+        if (write(fd, bmap, bmap_size) < 0) {
+            result = -errno;
+        }
+        g_free(bmap);
     }
-    if (write(fd, bmap, bmap_size) < 0) {
-        result = -errno;
-    }
-    g_free(bmap);
+
     if (image_type == VDI_TYPE_STATIC) {
         if (ftruncate(fd, sizeof(header) + bmap_size + blocks * block_size)) {
             result = -errno;
-- 
1.7.6.5

[Qemu-devel] [PATCH 06/11] block: fix block tray status

[Kevin Wolf]

From: Pavel Hrdina <phrdina@redhat.com>

The tray status should change also if you eject empty block device.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 470bdcc..c754353 100644
--- a/block.c
+++ b/block.c
@@ -897,10 +897,10 @@ void bdrv_close(BlockDriverState *bs)
             bdrv_delete(bs->file);
             bs->file = NULL;
         }
-
-        bdrv_dev_change_media_cb(bs, false);
     }
 
+    bdrv_dev_change_media_cb(bs, false);
+
     /*throttling disk I/O limits*/
     if (bs->io_limits_enabled) {
         bdrv_io_limits_disable(bs);
-- 
1.7.6.5

[Qemu-devel] [PATCH 07/11] ahci: properly reset PxCMD on HBA reset

[Kevin Wolf]

From: Jason Baron <jbaron@redhat.com>

While testing q35, I found that windows 7 (specifically, windows 7 ultimate
with sp1 x64), wouldn't install because it can't find the cdrom or disk drive.
The failure message is: 'A required cd/dvd device driver is missing. If you
have a driver floppy disk, CD, DVD, or USB flash drive, please insert it now.'
This can also be reproduced on piix by adding an ahci controller, and
observing that windows 7 does not see any devices behind it.

The problem is that when windows issues a HBA reset, qemu does not reset the
individual ports' PxCMD register. Windows 7 then reads back the PxCMD register
and presumably assumes that the ahci controller has already been initialized.
Windows then never sets up the PxIE register to enable interrupts, and thus it
never gets irqs back when it sends ata device inquiry commands.

This change brings qemu into ahci 1.3 specification compliance.

Section 10.4.3 HBA Reset:

"
When GHC.HR is set to '1', GHC.AE, GHC.IE, the IS register, and all port
register fields (except PxFB/PxFBU/PxCLB/PxCLBU) that are not HwInit in the
HBA's register memory space are reset.
"

I've also re-tested Fedora 16 and 17 to verify that they continue to work with
this change.

Signed-off-by: Jason Baron <jbaron@redhat.com>
Acked-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/ahci.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 5ea3cad..68671bc 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1175,7 +1175,6 @@ void ahci_init(AHCIState *s, DeviceState *qdev, DMAContext *dma, int ports)
         ad->port_no = i;
         ad->port.dma = &ad->dma;
         ad->port.dma->ops = &ahci_dma_ops;
-        ad->port_regs.cmd = PORT_CMD_SPIN_UP | PORT_CMD_POWER_ON;
     }
 }
 
@@ -1199,6 +1198,7 @@ void ahci_reset(AHCIState *s)
         pr->irq_stat = 0;
         pr->irq_mask = 0;
         pr->scr_ctl = 0;
+        pr->cmd = PORT_CMD_SPIN_UP | PORT_CMD_POWER_ON;
         ahci_reset_port(s, i);
     }
 }
-- 
1.7.6.5

[Qemu-devel] [PATCH 08/11] qapi: Add SnapshotInfo and ImageInfo.

[Kevin Wolf]

From: Benoît Canet <benoit@irqsave.net>

Signed-off-by: Benoit Canet <benoit@irqsave.net>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi-schema.json |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index a9f465a..1c168c5 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -156,6 +156,70 @@
             'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
 
 ##
+# @SnapshotInfo
+#
+# @id: unique snapshot id
+#
+# @name: user chosen name
+#
+# @vm-state-size: size of the VM state
+#
+# @date-sec: UTC date of the snapshot in seconds
+#
+# @date-nsec: fractional part in nano seconds to be used with date-sec
+#
+# @vm-clock-sec: VM clock relative to boot in seconds
+#
+# @vm-clock-nsec: fractional part in nano seconds to be used with vm-clock-sec
+#
+# Since: 1.3
+#
+##
+
+{ 'type': 'SnapshotInfo',
+  'data': { 'id': 'str', 'name': 'str', 'vm-state-size': 'int',
+            'date-sec': 'int', 'date-nsec': 'int',
+            'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }
+
+##
+# @ImageInfo:
+#
+# Information about a QEMU image file
+#
+# @filename: name of the image file
+#
+# @format: format of the image file
+#
+# @virtual-size: maximum capacity in bytes of the image
+#
+# @actual-size: #optional actual size on disk in bytes of the image
+#
+# @dirty-flag: #optional true if image is not cleanly closed
+#
+# @cluster-size: #optional size of a cluster in bytes
+#
+# @encrypted: #optional true if the image is encrypted
+#
+# @backing-filename: #optional name of the backing file
+#
+# @full-backing-filename: #optional full path of the backing file
+#
+# @backing-filename-format: #optional the format of the backing file
+#
+# @snapshots: #optional list of VM snapshots
+#
+# Since: 1.3
+#
+##
+
+{ 'type': 'ImageInfo',
+  'data': {'filename': 'str', 'format': 'str', '*dirty-flag': 'bool',
+           '*actual-size': 'int', 'virtual-size': 'int',
+           '*cluster-size': 'int', '*encrypted': 'bool',
+           '*backing-filename': 'str', '*full-backing-filename': 'str',
+           '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'] } }
+
+##
 # @StatusInfo:
 #
 # Information about VCPU run state
-- 
1.7.6.5

[Qemu-devel] [PATCH 09/11] qemu-img: Add json output option to the info command.

[Kevin Wolf]

From: Benoît Canet <benoit@irqsave.net>

This option --output=[human|json] make qemu-img info output on
human or JSON representation at the choice of the user.

example:
{
    "snapshots": [
        {
            "vm-clock-nsec": 637102488,
            "name": "vm-20120821145509",
            "date-sec": 1345553709,
            "date-nsec": 220289000,
            "vm-clock-sec": 20,
            "id": "1",
            "vm-state-size": 96522745
        },
        {
            "vm-clock-nsec": 28210866,
            "name": "vm-20120821154059",
            "date-sec": 1345556459,
            "date-nsec": 171392000,
            "vm-clock-sec": 46,
            "id": "2",
            "vm-state-size": 101208714
        }
    ],
    "virtual-size": 1073741824,
    "filename": "snap.qcow2",
    "cluster-size": 65536,
    "format": "qcow2",
    "actual-size": 985587712,
    "dirty-flag": false
}

Signed-off-by: Benoit Canet <benoit@irqsave.net>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 Makefile         |    3 +-
 qemu-img-cmds.hx |    4 +-
 qemu-img.c       |  236 ++++++++++++++++++++++++++++++++++++++++++++----------
 qemu-img.texi    |    5 +-
 4 files changed, 199 insertions(+), 49 deletions(-)

diff --git a/Makefile b/Makefile
index 1cd5bc8..971e92f 100644
--- a/Makefile
+++ b/Makefile
@@ -157,7 +157,8 @@ tools-obj-y = $(oslib-obj-y) $(trace-obj-y) qemu-tool.o qemu-timer.o \
 	iohandler.o cutils.o iov.o async.o
 tools-obj-$(CONFIG_POSIX) += compatfd.o
 
-qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y)
+qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y) $(qapi-obj-y) \
+                              qapi-visit.o qapi-types.o
 qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y) $(block-obj-y)
 qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y)
 
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 39419a0..0ef82e9 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -34,9 +34,9 @@ STEXI
 ETEXI
 
 DEF("info", img_info,
-    "info [-f fmt] filename")
+    "info [-f fmt] [--output=ofmt] filename")
 STEXI
-@item info [-f @var{fmt}] @var{filename}
+@item info [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
 ETEXI
 
 DEF("snapshot", img_snapshot,
diff --git a/qemu-img.c b/qemu-img.c
index b41e670..30e33c7 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -21,12 +21,16 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include "qapi-visit.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qjson.h"
 #include "qemu-common.h"
 #include "qemu-option.h"
 #include "qemu-error.h"
 #include "osdep.h"
 #include "sysemu.h"
 #include "block_int.h"
+#include <getopt.h>
 #include <stdio.h>
 
 #ifdef _WIN32
@@ -84,6 +88,7 @@ static void help(void)
            "  '-p' show progress of command (only certain commands)\n"
            "  '-S' indicates the consecutive number of bytes that must contain only zeros\n"
            "       for qemu-img to create a sparse image during conversion\n"
+           "  '--output' takes the format in which the output must be done (human or json)\n"
            "\n"
            "Parameters to check subcommand:\n"
            "  '-r' tries to repair any inconsistencies that are found during the check.\n"
@@ -1102,21 +1107,174 @@ static void dump_snapshots(BlockDriverState *bs)
     g_free(sn_tab);
 }
 
-static int img_info(int argc, char **argv)
+static void collect_snapshots(BlockDriverState *bs , ImageInfo *info)
+{
+    int i, sn_count;
+    QEMUSnapshotInfo *sn_tab = NULL;
+    SnapshotInfoList *info_list, *cur_item = NULL;
+    sn_count = bdrv_snapshot_list(bs, &sn_tab);
+
+    for (i = 0; i < sn_count; i++) {
+        info->has_snapshots = true;
+        info_list = g_new0(SnapshotInfoList, 1);
+
+        info_list->value                = g_new0(SnapshotInfo, 1);
+        info_list->value->id            = g_strdup(sn_tab[i].id_str);
+        info_list->value->name          = g_strdup(sn_tab[i].name);
+        info_list->value->vm_state_size = sn_tab[i].vm_state_size;
+        info_list->value->date_sec      = sn_tab[i].date_sec;
+        info_list->value->date_nsec     = sn_tab[i].date_nsec;
+        info_list->value->vm_clock_sec  = sn_tab[i].vm_clock_nsec / 1000000000;
+        info_list->value->vm_clock_nsec = sn_tab[i].vm_clock_nsec % 1000000000;
+
+        /* XXX: waiting for the qapi to support qemu-queue.h types */
+        if (!cur_item) {
+            info->snapshots = cur_item = info_list;
+        } else {
+            cur_item->next = info_list;
+            cur_item = info_list;
+        }
+
+    }
+
+    g_free(sn_tab);
+}
+
+static void dump_json_image_info(ImageInfo *info)
+{
+    Error *errp = NULL;
+    QString *str;
+    QmpOutputVisitor *ov = qmp_output_visitor_new();
+    QObject *obj;
+    visit_type_ImageInfo(qmp_output_get_visitor(ov),
+                         &info, NULL, &errp);
+    obj = qmp_output_get_qobject(ov);
+    str = qobject_to_json_pretty(obj);
+    assert(str != NULL);
+    printf("%s\n", qstring_get_str(str));
+    qobject_decref(obj);
+    qmp_output_visitor_cleanup(ov);
+    QDECREF(str);
+}
+
+static void collect_image_info(BlockDriverState *bs,
+                   ImageInfo *info,
+                   const char *filename,
+                   const char *fmt)
 {
-    int c;
-    const char *filename, *fmt;
-    BlockDriverState *bs;
-    char size_buf[128], dsize_buf[128];
     uint64_t total_sectors;
-    int64_t allocated_size;
     char backing_filename[1024];
     char backing_filename2[1024];
     BlockDriverInfo bdi;
 
+    bdrv_get_geometry(bs, &total_sectors);
+
+    info->filename        = g_strdup(filename);
+    info->format          = g_strdup(bdrv_get_format_name(bs));
+    info->virtual_size    = total_sectors * 512;
+    info->actual_size     = bdrv_get_allocated_file_size(bs);
+    info->has_actual_size = info->actual_size >= 0;
+    if (bdrv_is_encrypted(bs)) {
+        info->encrypted = true;
+        info->has_encrypted = true;
+    }
+    if (bdrv_get_info(bs, &bdi) >= 0) {
+        if (bdi.cluster_size != 0) {
+            info->cluster_size = bdi.cluster_size;
+            info->has_cluster_size = true;
+        }
+        info->dirty_flag = bdi.is_dirty;
+        info->has_dirty_flag = true;
+    }
+    bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
+    if (backing_filename[0] != '\0') {
+        info->backing_filename = g_strdup(backing_filename);
+        info->has_backing_filename = true;
+        bdrv_get_full_backing_filename(bs, backing_filename2,
+                                       sizeof(backing_filename2));
+
+        if (strcmp(backing_filename, backing_filename2) != 0) {
+            info->full_backing_filename =
+                        g_strdup(backing_filename2);
+            info->has_full_backing_filename = true;
+        }
+
+        if (bs->backing_format[0]) {
+            info->backing_filename_format = g_strdup(bs->backing_format);
+            info->has_backing_filename_format = true;
+        }
+    }
+}
+
+static void dump_human_image_info(ImageInfo *info)
+{
+    char size_buf[128], dsize_buf[128];
+    if (!info->has_actual_size) {
+        snprintf(dsize_buf, sizeof(dsize_buf), "unavailable");
+    } else {
+        get_human_readable_size(dsize_buf, sizeof(dsize_buf),
+                                info->actual_size);
+    }
+    get_human_readable_size(size_buf, sizeof(size_buf), info->virtual_size);
+    printf("image: %s\n"
+           "file format: %s\n"
+           "virtual size: %s (%" PRId64 " bytes)\n"
+           "disk size: %s\n",
+           info->filename, info->format, size_buf,
+           info->virtual_size,
+           dsize_buf);
+
+    if (info->has_encrypted && info->encrypted) {
+        printf("encrypted: yes\n");
+    }
+
+    if (info->has_cluster_size) {
+        printf("cluster_size: %" PRId64 "\n", info->cluster_size);
+    }
+
+    if (info->has_dirty_flag && info->dirty_flag) {
+        printf("cleanly shut down: no\n");
+    }
+
+    if (info->has_backing_filename) {
+        printf("backing file: %s", info->backing_filename);
+        if (info->has_full_backing_filename) {
+            printf(" (actual path: %s)", info->full_backing_filename);
+        }
+        putchar('\n');
+        if (info->has_backing_filename_format) {
+            printf("backing file format: %s\n", info->backing_filename_format);
+        }
+    }
+}
+
+enum {OPTION_OUTPUT = 256};
+
+typedef enum OutputFormat {
+    OFORMAT_JSON,
+    OFORMAT_HUMAN,
+} OutputFormat;
+
+static int img_info(int argc, char **argv)
+{
+    int c;
+    OutputFormat output_format = OFORMAT_HUMAN;
+    const char *filename, *fmt, *output;
+    BlockDriverState *bs;
+    ImageInfo *info;
+
     fmt = NULL;
+    output = NULL;
     for(;;) {
-        c = getopt(argc, argv, "f:h");
+        int option_index = 0;
+        static const struct option long_options[] = {
+            {"help", no_argument, 0, 'h'},
+            {"format", required_argument, 0, 'f'},
+            {"output", required_argument, 0, OPTION_OUTPUT},
+            {0, 0, 0, 0}
+        };
+        c = getopt_long(argc, argv, "f:h",
+                        long_options, &option_index);
         if (c == -1) {
             break;
         }
@@ -1128,6 +1286,9 @@ static int img_info(int argc, char **argv)
         case 'f':
             fmt = optarg;
             break;
+        case OPTION_OUTPUT:
+            output = optarg;
+            break;
         }
     }
     if (optind >= argc) {
@@ -1135,48 +1296,35 @@ static int img_info(int argc, char **argv)
     }
     filename = argv[optind++];
 
+    if (output && !strcmp(output, "json")) {
+        output_format = OFORMAT_JSON;
+    } else if (output && !strcmp(output, "human")) {
+        output_format = OFORMAT_HUMAN;
+    } else if (output) {
+        error_report("--output must be used with human or json as argument.");
+        return 1;
+    }
+
     bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING);
     if (!bs) {
         return 1;
     }
-    bdrv_get_geometry(bs, &total_sectors);
-    get_human_readable_size(size_buf, sizeof(size_buf), total_sectors * 512);
-    allocated_size = bdrv_get_allocated_file_size(bs);
-    if (allocated_size < 0) {
-        snprintf(dsize_buf, sizeof(dsize_buf), "unavailable");
-    } else {
-        get_human_readable_size(dsize_buf, sizeof(dsize_buf),
-                                allocated_size);
-    }
-    printf("image: %s\n"
-           "file format: %s\n"
-           "virtual size: %s (%" PRId64 " bytes)\n"
-           "disk size: %s\n",
-           filename, bdrv_get_format_name(bs), size_buf,
-           (total_sectors * 512),
-           dsize_buf);
-    if (bdrv_is_encrypted(bs)) {
-        printf("encrypted: yes\n");
-    }
-    if (bdrv_get_info(bs, &bdi) >= 0) {
-        if (bdi.cluster_size != 0) {
-            printf("cluster_size: %d\n", bdi.cluster_size);
-        }
-        if (bdi.is_dirty) {
-            printf("cleanly shut down: no\n");
-        }
-    }
-    bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
-    if (backing_filename[0] != '\0') {
-        bdrv_get_full_backing_filename(bs, backing_filename2,
-                                       sizeof(backing_filename2));
-        printf("backing file: %s", backing_filename);
-        if (strcmp(backing_filename, backing_filename2) != 0) {
-            printf(" (actual path: %s)", backing_filename2);
-        }
-        putchar('\n');
+
+    info = g_new0(ImageInfo, 1);
+    collect_image_info(bs, info, filename, fmt);
+
+    switch (output_format) {
+    case OFORMAT_HUMAN:
+        dump_human_image_info(info);
+        dump_snapshots(bs);
+        break;
+    case OFORMAT_JSON:
+        collect_snapshots(bs, info);
+        dump_json_image_info(info);
+        break;
     }
-    dump_snapshots(bs);
+
+    qapi_free_ImageInfo(info);
     bdrv_delete(bs);
     return 0;
 }
diff --git a/qemu-img.texi b/qemu-img.texi
index 6b42e35..ca85891 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -129,12 +129,13 @@ created as a copy on write image of the specified base image; the
 @var{backing_file} should have the same content as the input's base image,
 however the path, image format, etc may differ.
 
-@item info [-f @var{fmt}] @var{filename}
+@item info [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
 
 Give information about the disk image @var{filename}. Use it in
 particular to know the size reserved on disk which can be different
 from the displayed size. If VM snapshots are stored in the disk image,
-they are displayed too.
+they are displayed too. The command can output in the format @var{ofmt}
+which is either @code{human} or @code{json}.
 
 @item snapshot [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot} ] @var{filename}
 
-- 
1.7.6.5

[Qemu-devel] [PATCH 10/11] Don't require encryption password for 'qemu-img info' command

[Kevin Wolf]

From: "Daniel P. Berrange" <berrange@redhat.com>

The encryption password is only required if I/O is going to be
performed on a disk image. The 'qemu-img info' command merely
reports metadata, so it should not ask for a decryption password

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-img.c |   21 +++++++++++----------
 1 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 30e33c7..a374d67 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -226,7 +226,8 @@ static int print_block_option_help(const char *filename, const char *fmt)
 
 static BlockDriverState *bdrv_new_open(const char *filename,
                                        const char *fmt,
-                                       int flags)
+                                       int flags,
+                                       bool require_io)
 {
     BlockDriverState *bs;
     BlockDriver *drv;
@@ -251,7 +252,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
         goto fail;
     }
 
-    if (bdrv_is_encrypted(bs)) {
+    if (bdrv_is_encrypted(bs) && require_io) {
         printf("Disk image '%s' is encrypted.\n", filename);
         if (read_password(password, sizeof(password)) < 0) {
             error_report("No password given");
@@ -418,7 +419,7 @@ static int img_check(int argc, char **argv)
     }
     filename = argv[optind++];
 
-    bs = bdrv_new_open(filename, fmt, flags);
+    bs = bdrv_new_open(filename, fmt, flags, true);
     if (!bs) {
         return 1;
     }
@@ -525,7 +526,7 @@ static int img_commit(int argc, char **argv)
         return -1;
     }
 
-    bs = bdrv_new_open(filename, fmt, flags);
+    bs = bdrv_new_open(filename, fmt, flags, true);
     if (!bs) {
         return 1;
     }
@@ -767,7 +768,7 @@ static int img_convert(int argc, char **argv)
 
     total_sectors = 0;
     for (bs_i = 0; bs_i < bs_n; bs_i++) {
-        bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, BDRV_O_FLAGS);
+        bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, BDRV_O_FLAGS, true);
         if (!bs[bs_i]) {
             error_report("Could not open '%s'", argv[optind + bs_i]);
             ret = -1;
@@ -886,7 +887,7 @@ static int img_convert(int argc, char **argv)
         return -1;
     }
 
-    out_bs = bdrv_new_open(out_filename, out_fmt, flags);
+    out_bs = bdrv_new_open(out_filename, out_fmt, flags, true);
     if (!out_bs) {
         ret = -1;
         goto out;
@@ -1305,7 +1306,7 @@ static int img_info(int argc, char **argv)
         return 1;
     }
 
-    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING);
+    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING, false);
     if (!bs) {
         return 1;
     }
@@ -1396,7 +1397,7 @@ static int img_snapshot(int argc, char **argv)
     filename = argv[optind++];
 
     /* Open the image */
-    bs = bdrv_new_open(filename, NULL, bdrv_oflags);
+    bs = bdrv_new_open(filename, NULL, bdrv_oflags, true);
     if (!bs) {
         return 1;
     }
@@ -1514,7 +1515,7 @@ static int img_rebase(int argc, char **argv)
      * Ignore the old backing file for unsafe rebase in case we want to correct
      * the reference to a renamed or moved backing file.
      */
-    bs = bdrv_new_open(filename, fmt, flags);
+    bs = bdrv_new_open(filename, fmt, flags, true);
     if (!bs) {
         return 1;
     }
@@ -1787,7 +1788,7 @@ static int img_resize(int argc, char **argv)
     n = qemu_opt_get_size(param, BLOCK_OPT_SIZE, 0);
     qemu_opts_del(param);
 
-    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR);
+    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR, true);
     if (!bs) {
         ret = -1;
         goto out;
-- 
1.7.6.5

[Qemu-devel] [PATCH 11/11] block: Don't forget to delete temporary file

[Kevin Wolf]

From: Dunrong Huang <riegamaths@gmail.com>

The caller would not delete temporary file after failed get_tmp_filename().

Signed-off-by: Dunrong Huang <riegamaths@gmail.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/block.c b/block.c
index c754353..e78039b 100644
--- a/block.c
+++ b/block.c
@@ -433,7 +433,11 @@ int get_tmp_filename(char *filename, int size)
         return -EOVERFLOW;
     }
     fd = mkstemp(filename);
-    if (fd < 0 || close(fd)) {
+    if (fd < 0) {
+        return -errno;
+    }
+    if (close(fd) != 0) {
+        unlink(filename);
         return -errno;
     }
     return 0;
-- 
1.7.6.5

Re: [Qemu-devel] [PULL 00/11] Block patches

[Anthony Liguori]

Kevin Wolf <kwolf@redhat.com> writes:

> The following changes since commit e0a1e32dbc41e6b2aabb436a9417dfd32177a3dc:
>
>   Merge branch 'usb.64' of git://git.kraxel.org/qemu (2012-09-11 18:06:56 +0200)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/kevin.git for-anthony
>

Pulled. Thanks.

Regards,

Anthony Liguori

> Benoît Canet (2):
>       qapi: Add SnapshotInfo and ImageInfo.
>       qemu-img: Add json output option to the info command.
>
> Daniel P. Berrange (1):
>       Don't require encryption password for 'qemu-img info' command
>
> Dunrong Huang (1):
>       block: Don't forget to delete temporary file
>
> Jason Baron (1):
>       ahci: properly reset PxCMD on HBA reset
>
> MORITA Kazutaka (1):
>       sheepdog: fix savevm and loadvm
>
> Pavel Hrdina (1):
>       block: fix block tray status
>
> Ronnie Sahlberg (1):
>       ATAPI: STARTSTOPUNIT only eject/load media if powercondition is 0
>
> Stefan Weil (3):
>       ide: Fix error messages from static code analysis (no real error)
>       block/curl: Fix wrong free statement
>       vdi: Fix warning from clang
>
>  Makefile         |    3 +-
>  block.c          |   10 ++-
>  block/curl.c     |    3 +-
>  block/sheepdog.c |    3 +-
>  block/vdi.c      |   25 +++---
>  hw/ide/ahci.c    |    2 +-
>  hw/ide/atapi.c   |    6 ++
>  hw/ide/core.c    |   11 +--
>  qapi-schema.json |   64 ++++++++++++++
>  qemu-img-cmds.hx |    4 +-
>  qemu-img.c       |  257 ++++++++++++++++++++++++++++++++++++++++++-----------
>  qemu-img.texi    |    5 +-
>  12 files changed, 305 insertions(+), 88 deletions(-)

Re: [Qemu-devel] [PULL 00/11] Block patches

[Michael Tokarev]

On 14.09.2012 16:39, Kevin Wolf wrote:
> The following changes since commit e0a1e32dbc41e6b2aabb436a9417dfd32177a3dc:
> 
>   Merge branch 'usb.64' of git://git.kraxel.org/qemu (2012-09-11 18:06:56 +0200)
> 
> are available in the git repository at:
> 
>   git://repo.or.cz/qemu/kevin.git for-anthony

Are any of these appropriate for -stable?  I think some are, below:

> Benoît Canet (2):
>       qapi: Add SnapshotInfo and ImageInfo.
>       qemu-img: Add json output option to the info command.
> 
> Daniel P. Berrange (1):
>       Don't require encryption password for 'qemu-img info' command
> 
> Dunrong Huang (1):
>       block: Don't forget to delete temporary file

This?  Appears to be a minor case.

> Jason Baron (1):
>       ahci: properly reset PxCMD on HBA reset

Most likely this.  Jason?

> MORITA Kazutaka (1):
>       sheepdog: fix savevm and loadvm

This, but does anyone really use sheepdog with savevm/loadvm?
Probably a very minor issue too, not worth -stable.

> Pavel Hrdina (1):
>       block: fix block tray status

And this,

> Ronnie Sahlberg (1):
>       ATAPI: STARTSTOPUNIT only eject/load media if powercondition is 0

And probably this too?  I'm not sure about this one.
Ronnie?

> Stefan Weil (3):
>       ide: Fix error messages from static code analysis (no real error)
>       block/curl: Fix wrong free statement
>       vdi: Fix warning from clang

Thanks,

/mjt

Re: [Qemu-devel] [PULL 00/11] Block patches

[Kevin Wolf]

Am 18.09.2012 19:49, schrieb Michael Tokarev:
> On 14.09.2012 16:39, Kevin Wolf wrote:
>> The following changes since commit e0a1e32dbc41e6b2aabb436a9417dfd32177a3dc:
>>
>>   Merge branch 'usb.64' of git://git.kraxel.org/qemu (2012-09-11 18:06:56 +0200)
>>
>> are available in the git repository at:
>>
>>   git://repo.or.cz/qemu/kevin.git for-anthony
> 
> Are any of these appropriate for -stable?  I think some are, below:

I think all of your suggestions would be appropriate, even though most
of them fix rather theoretical cases.

>> Jason Baron (1):
>>       ahci: properly reset PxCMD on HBA reset
> 
> Most likely this.  Jason?

Yes, this fixes a real visible bug with AHCI CD-ROMs.

Kevin