* [Qemu-devel] [PATCH 1/8] Documentation: Remove outdated host_device note
2011-07-06 14:21 [Qemu-devel] [PULL 0/8] Block patches Kevin Wolf
@ 2011-07-06 14:21 ` Kevin Wolf
2011-07-06 14:21 ` [Qemu-devel] [PATCH 2/8] qemu-img: Add cache command line option Kevin Wolf
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2011-07-06 14:21 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
People shouldn't explicitly specify host_device any more. raw is doing the
Right Thing.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qemu-img.texi | 6 ------
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/qemu-img.texi b/qemu-img.texi
index ced64a4..526474c 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -173,12 +173,6 @@ Linux or NTFS on Windows), then only the written sectors will reserve
space. Use @code{qemu-img info} to know the real size used by the
image or @code{ls -ls} on Unix/Linux.
-@item host_device
-
-Host device format. This format should be used instead of raw when
-converting to block devices or other devices where "holes" are not
-supported.
-
@item qcow2
QEMU image format, the most versatile format. Use it to have smaller
images (useful if your filesystem does not supports holes, for example
--
1.7.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [PATCH 2/8] qemu-img: Add cache command line option
2011-07-06 14:21 [Qemu-devel] [PULL 0/8] Block patches Kevin Wolf
2011-07-06 14:21 ` [Qemu-devel] [PATCH 1/8] Documentation: Remove outdated host_device note Kevin Wolf
@ 2011-07-06 14:21 ` Kevin Wolf
2011-07-06 14:21 ` [Qemu-devel] [PATCH 3/8] block/raw-posix: Linux compat-ioctl warning workaround Kevin Wolf
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2011-07-06 14:21 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Federico Simoncelli <fsimonce@redhat.com>
qemu-img currently writes disk images using writeback and filling
up the cache buffers which are then flushed by the kernel preventing
other processes from accessing the storage.
This is particularly bad in cluster environments where time-based
algorithms might be in place and accessing the storage within
certain timeouts is critical.
This patch adds the option to choose a cache method when writing
disk images.
Signed-off-by: Federico Simoncelli <fsimonce@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qemu-img-cmds.hx | 6 ++--
qemu-img.c | 80 +++++++++++++++++++++++++++++++++++++++++++++---------
2 files changed, 70 insertions(+), 16 deletions(-)
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 3072d38..2b70618 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -22,13 +22,13 @@ STEXI
ETEXI
DEF("commit", img_commit,
- "commit [-f fmt] filename")
+ "commit [-f fmt] [-t cache] filename")
STEXI
@item commit [-f @var{fmt}] @var{filename}
ETEXI
DEF("convert", img_convert,
- "convert [-c] [-p] [-f fmt] [-O output_fmt] [-o options] [-s snapshot_name] filename [filename2 [...]] output_filename")
+ "convert [-c] [-p] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_name] filename [filename2 [...]] output_filename")
STEXI
@item convert [-c] [-f @var{fmt}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] @var{filename} [@var{filename2} [...]] @var{output_filename}
ETEXI
@@ -46,7 +46,7 @@ STEXI
ETEXI
DEF("rebase", img_rebase,
- "rebase [-f fmt] [-p] [-u] -b backing_file [-F backing_fmt] filename")
+ "rebase [-f fmt] [-t cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
STEXI
@item rebase [-f @var{fmt}] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
ETEXI
diff --git a/qemu-img.c b/qemu-img.c
index 32628b3..54137a4 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -40,6 +40,7 @@ typedef struct img_cmd_t {
/* Default to cache=writeback as data integrity is not important for qemu-tcg. */
#define BDRV_O_FLAGS BDRV_O_CACHE_WB
+#define BDRV_DEFAULT_CACHE "writeback"
static void format_print(void *opaque, const char *name)
{
@@ -64,6 +65,8 @@ static void help(void)
"Command parameters:\n"
" 'filename' is a disk image filename\n"
" 'fmt' is the disk image format. It is guessed automatically in most cases\n"
+ " 'cache' is the cache mode used to write the output disk image, the valid\n"
+ " options are: 'none', 'writeback' (default), 'writethrough' and 'unsafe'\n"
" 'size' is the disk image size in bytes. Optional suffixes\n"
" 'k' or 'K' (kilobyte, 1024), 'M' (megabyte, 1024k), 'G' (gigabyte, 1024M)\n"
" and T (terabyte, 1024G) are supported. 'b' is ignored.\n"
@@ -180,6 +183,27 @@ static int read_password(char *buf, int buf_size)
}
#endif
+static int set_cache_flag(const char *mode, int *flags)
+{
+ *flags &= ~BDRV_O_CACHE_MASK;
+
+ if (!strcmp(mode, "none") || !strcmp(mode, "off")) {
+ *flags |= BDRV_O_CACHE_WB;
+ *flags |= BDRV_O_NOCACHE;
+ } else if (!strcmp(mode, "writeback")) {
+ *flags |= BDRV_O_CACHE_WB;
+ } else if (!strcmp(mode, "unsafe")) {
+ *flags |= BDRV_O_CACHE_WB;
+ *flags |= BDRV_O_NO_FLUSH;
+ } else if (!strcmp(mode, "writethrough")) {
+ /* this is the default */
+ } else {
+ return -1;
+ }
+
+ return 0;
+}
+
static int print_block_option_help(const char *filename, const char *fmt)
{
BlockDriver *drv, *proto_drv;
@@ -441,13 +465,14 @@ static int img_check(int argc, char **argv)
static int img_commit(int argc, char **argv)
{
- int c, ret;
- const char *filename, *fmt;
+ int c, ret, flags;
+ const char *filename, *fmt, *cache;
BlockDriverState *bs;
fmt = NULL;
+ cache = BDRV_DEFAULT_CACHE;
for(;;) {
- c = getopt(argc, argv, "f:h");
+ c = getopt(argc, argv, "f:ht:");
if (c == -1) {
break;
}
@@ -459,6 +484,9 @@ static int img_commit(int argc, char **argv)
case 'f':
fmt = optarg;
break;
+ case 't':
+ cache = optarg;
+ break;
}
}
if (optind >= argc) {
@@ -466,7 +494,14 @@ static int img_commit(int argc, char **argv)
}
filename = argv[optind++];
- bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR);
+ flags = BDRV_O_RDWR;
+ ret = set_cache_flag(cache, &flags);
+ if (ret < 0) {
+ error_report("Invalid cache option: %s", cache);
+ return -1;
+ }
+
+ bs = bdrv_new_open(filename, fmt, flags);
if (!bs) {
return 1;
}
@@ -591,8 +626,8 @@ static int compare_sectors(const uint8_t *buf1, const uint8_t *buf2, int n,
static int img_convert(int argc, char **argv)
{
int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, cluster_sectors;
- int progress = 0;
- const char *fmt, *out_fmt, *out_baseimg, *out_filename;
+ int progress = 0, flags;
+ const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename;
BlockDriver *drv, *proto_drv;
BlockDriverState **bs = NULL, *out_bs = NULL;
int64_t total_sectors, nb_sectors, sector_num, bs_offset;
@@ -608,10 +643,11 @@ static int img_convert(int argc, char **argv)
fmt = NULL;
out_fmt = "raw";
+ cache = "unsafe";
out_baseimg = NULL;
compress = 0;
for(;;) {
- c = getopt(argc, argv, "f:O:B:s:hce6o:p");
+ c = getopt(argc, argv, "f:O:B:s:hce6o:pt:");
if (c == -1) {
break;
}
@@ -649,6 +685,9 @@ static int img_convert(int argc, char **argv)
case 'p':
progress = 1;
break;
+ case 't':
+ cache = optarg;
+ break;
}
}
@@ -779,8 +818,14 @@ static int img_convert(int argc, char **argv)
goto out;
}
- out_bs = bdrv_new_open(out_filename, out_fmt,
- BDRV_O_FLAGS | BDRV_O_RDWR | BDRV_O_NO_FLUSH);
+ flags = BDRV_O_RDWR;
+ ret = set_cache_flag(cache, &flags);
+ if (ret < 0) {
+ error_report("Invalid cache option: %s", cache);
+ return -1;
+ }
+
+ out_bs = bdrv_new_open(out_filename, out_fmt, flags);
if (!out_bs) {
ret = -1;
goto out;
@@ -1225,18 +1270,18 @@ static int img_rebase(int argc, char **argv)
BlockDriverState *bs, *bs_old_backing = NULL, *bs_new_backing = NULL;
BlockDriver *old_backing_drv, *new_backing_drv;
char *filename;
- const char *fmt, *out_basefmt, *out_baseimg;
+ const char *fmt, *cache, *out_basefmt, *out_baseimg;
int c, flags, ret;
int unsafe = 0;
int progress = 0;
/* Parse commandline parameters */
fmt = NULL;
+ cache = BDRV_DEFAULT_CACHE;
out_baseimg = NULL;
out_basefmt = NULL;
-
for(;;) {
- c = getopt(argc, argv, "uhf:F:b:p");
+ c = getopt(argc, argv, "uhf:F:b:pt:");
if (c == -1) {
break;
}
@@ -1260,6 +1305,9 @@ static int img_rebase(int argc, char **argv)
case 'p':
progress = 1;
break;
+ case 't':
+ cache = optarg;
+ break;
}
}
@@ -1271,13 +1319,19 @@ static int img_rebase(int argc, char **argv)
qemu_progress_init(progress, 2.0);
qemu_progress_print(0, 100);
+ flags = BDRV_O_RDWR | (unsafe ? BDRV_O_NO_BACKING : 0);
+ ret = set_cache_flag(cache, &flags);
+ if (ret < 0) {
+ error_report("Invalid cache option: %s", cache);
+ return -1;
+ }
+
/*
* Open the images.
*
* Ignore the old backing file for unsafe rebase in case we want to correct
* the reference to a renamed or moved backing file.
*/
- flags = BDRV_O_FLAGS | BDRV_O_RDWR | (unsafe ? BDRV_O_NO_BACKING : 0);
bs = bdrv_new_open(filename, fmt, flags);
if (!bs) {
return 1;
--
1.7.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [PATCH 3/8] block/raw-posix: Linux compat-ioctl warning workaround
2011-07-06 14:21 [Qemu-devel] [PULL 0/8] Block patches Kevin Wolf
2011-07-06 14:21 ` [Qemu-devel] [PATCH 1/8] Documentation: Remove outdated host_device note Kevin Wolf
2011-07-06 14:21 ` [Qemu-devel] [PATCH 2/8] qemu-img: Add cache command line option Kevin Wolf
@ 2011-07-06 14:21 ` Kevin Wolf
2011-07-06 14:21 ` [Qemu-devel] [PATCH 4/8] virtio-blk: Turn drive serial into a qdev property Kevin Wolf
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2011-07-06 14:21 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Johannes Stezenbach <js@sig21.net>
On Linux x86_64 host with 32bit userspace, running
qemu or even just "qemu-img create -f qcow2 some.img 1G"
causes a kernel warning:
ioctl32(qemu-img:5296): Unknown cmd fd(3) cmd(00005326){t:'S';sz:0} arg(7fffffff) on some.img
ioctl32(qemu-img:5296): Unknown cmd fd(3) cmd(801c0204){t:02;sz:28} arg(fff77350) on some.img
ioctl 00005326 is CDROM_DRIVE_STATUS,
ioctl 801c0204 is FDGETPRM.
The warning appears because the Linux compat-ioctl handler for these
ioctls only applies to block devices, while qemu also uses the ioctls on
plain files. Work around by calling fstat() the ensure the ioctls are
only used on block devices.
Signed-off-by: Johannes Stezenbach <js@sig21.net>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/raw-posix.c | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 4cd7d7a..34b64aa 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -46,6 +46,8 @@
#include <sys/dkio.h>
#endif
#ifdef __linux__
+#include <sys/types.h>
+#include <sys/stat.h>
#include <sys/ioctl.h>
#include <sys/param.h>
#include <linux/cdrom.h>
@@ -1188,6 +1190,7 @@ static int floppy_probe_device(const char *filename)
int fd, ret;
int prio = 0;
struct floppy_struct fdparam;
+ struct stat st;
if (strstart(filename, "/dev/fd", NULL))
prio = 50;
@@ -1196,12 +1199,17 @@ static int floppy_probe_device(const char *filename)
if (fd < 0) {
goto out;
}
+ ret = fstat(fd, &st);
+ if (ret == -1 || !S_ISBLK(st.st_mode)) {
+ goto outc;
+ }
/* Attempt to detect via a floppy specific ioctl */
ret = ioctl(fd, FDGETPRM, &fdparam);
if (ret >= 0)
prio = 100;
+outc:
close(fd);
out:
return prio;
@@ -1290,17 +1298,23 @@ static int cdrom_probe_device(const char *filename)
{
int fd, ret;
int prio = 0;
+ struct stat st;
fd = open(filename, O_RDONLY | O_NONBLOCK);
if (fd < 0) {
goto out;
}
+ ret = fstat(fd, &st);
+ if (ret == -1 || !S_ISBLK(st.st_mode)) {
+ goto outc;
+ }
/* Attempt to detect via a CDROM specific ioctl */
ret = ioctl(fd, CDROM_DRIVE_STATUS, CDSL_CURRENT);
if (ret >= 0)
prio = 100;
+outc:
close(fd);
out:
return prio;
--
1.7.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [PATCH 4/8] virtio-blk: Turn drive serial into a qdev property
2011-07-06 14:21 [Qemu-devel] [PULL 0/8] Block patches Kevin Wolf
` (2 preceding siblings ...)
2011-07-06 14:21 ` [Qemu-devel] [PATCH 3/8] block/raw-posix: Linux compat-ioctl warning workaround Kevin Wolf
@ 2011-07-06 14:21 ` Kevin Wolf
2011-07-06 14:21 ` [Qemu-devel] [PATCH 5/8] block: drive_init(): Simplify interface type setting Kevin Wolf
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2011-07-06 14:21 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
It needs to be a qdev property, because it belongs to the drive's
guest part. Precedence: commit a0fef654 and 6ced55a5.
Bonus: info qtree now shows the serial number.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
hw/s390-virtio-bus.c | 4 +++-
hw/s390-virtio-bus.h | 1 +
hw/virtio-blk.c | 29 +++++++++++++++++++----------
hw/virtio-blk.h | 2 ++
hw/virtio-pci.c | 4 +++-
hw/virtio-pci.h | 1 +
hw/virtio.h | 3 ++-
7 files changed, 31 insertions(+), 13 deletions(-)
diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index d4a12f7..2bf4821 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -128,7 +128,8 @@ static int s390_virtio_blk_init(VirtIOS390Device *dev)
{
VirtIODevice *vdev;
- vdev = virtio_blk_init((DeviceState *)dev, &dev->block);
+ vdev = virtio_blk_init((DeviceState *)dev, &dev->block,
+ &dev->block_serial);
if (!vdev) {
return -1;
}
@@ -355,6 +356,7 @@ static VirtIOS390DeviceInfo s390_virtio_blk = {
.qdev.size = sizeof(VirtIOS390Device),
.qdev.props = (Property[]) {
DEFINE_BLOCK_PROPERTIES(VirtIOS390Device, block),
+ DEFINE_PROP_STRING("serial", VirtIOS390Device, block_serial),
DEFINE_PROP_END_OF_LIST(),
},
};
diff --git a/hw/s390-virtio-bus.h b/hw/s390-virtio-bus.h
index 0c412d0..f1bece7 100644
--- a/hw/s390-virtio-bus.h
+++ b/hw/s390-virtio-bus.h
@@ -42,6 +42,7 @@ typedef struct VirtIOS390Device {
uint8_t feat_len;
VirtIODevice *vdev;
BlockConf block;
+ char *block_serial;
NICConf nic;
uint32_t host_features;
virtio_serial_conf serial;
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 91e0394..6471ac8 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -28,8 +28,8 @@ typedef struct VirtIOBlock
void *rq;
QEMUBH *bh;
BlockConf *conf;
+ char *serial;
unsigned short sector_mask;
- char sn[BLOCK_SERIAL_STRLEN];
DeviceState *qdev;
} VirtIOBlock;
@@ -362,8 +362,13 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req,
} else if (type & VIRTIO_BLK_T_GET_ID) {
VirtIOBlock *s = req->dev;
- memcpy(req->elem.in_sg[0].iov_base, s->sn,
- MIN(req->elem.in_sg[0].iov_len, sizeof(s->sn)));
+ /*
+ * NB: per existing s/n string convention the string is
+ * terminated by '\0' only when shorter than buffer.
+ */
+ strncpy(req->elem.in_sg[0].iov_base,
+ s->serial ? s->serial : "",
+ MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
} else if (type & VIRTIO_BLK_T_OUT) {
qemu_iovec_init_external(&req->qiov, &req->elem.out_sg[1],
@@ -531,7 +536,8 @@ static void virtio_blk_change_cb(void *opaque, int reason)
}
}
-VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf)
+VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf,
+ char **serial)
{
VirtIOBlock *s;
int cylinders, heads, secs;
@@ -547,6 +553,14 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf)
return NULL;
}
+ if (!*serial) {
+ /* try to fall back to value set with legacy -drive serial=... */
+ dinfo = drive_get_by_blockdev(conf->bs);
+ if (*dinfo->serial) {
+ *serial = strdup(dinfo->serial);
+ }
+ }
+
s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
sizeof(struct virtio_blk_config),
sizeof(VirtIOBlock));
@@ -556,16 +570,11 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf)
s->vdev.reset = virtio_blk_reset;
s->bs = conf->bs;
s->conf = conf;
+ s->serial = *serial;
s->rq = NULL;
s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
- /* NB: per existing s/n string convention the string is terminated
- * by '\0' only when less than sizeof (s->sn)
- */
- dinfo = drive_get_by_blockdev(s->bs);
- strncpy(s->sn, dinfo->serial, sizeof (s->sn));
-
s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output);
qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s);
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index fff46da..5645d2b 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -34,6 +34,8 @@
#define VIRTIO_BLK_F_WCACHE 9 /* write cache enabled */
#define VIRTIO_BLK_F_TOPOLOGY 10 /* Topology information is available */
+#define VIRTIO_BLK_ID_BYTES 20 /* ID string length */
+
struct virtio_blk_config
{
uint64_t capacity;
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index b3e7ba5..d685243 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -700,7 +700,8 @@ static int virtio_blk_init_pci(PCIDevice *pci_dev)
proxy->class_code != PCI_CLASS_STORAGE_OTHER)
proxy->class_code = PCI_CLASS_STORAGE_SCSI;
- vdev = virtio_blk_init(&pci_dev->qdev, &proxy->block);
+ vdev = virtio_blk_init(&pci_dev->qdev, &proxy->block,
+ &proxy->block_serial);
if (!vdev) {
return -1;
}
@@ -805,6 +806,7 @@ static PCIDeviceInfo virtio_info[] = {
.qdev.props = (Property[]) {
DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, block),
+ DEFINE_PROP_STRING("serial", VirtIOPCIProxy, block_serial),
DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
index b518917..1f0de56 100644
--- a/hw/virtio-pci.h
+++ b/hw/virtio-pci.h
@@ -26,6 +26,7 @@ typedef struct {
uint32_t class_code;
uint32_t nvectors;
BlockConf block;
+ char *block_serial;
NICConf nic;
uint32_t host_features;
#ifdef CONFIG_LINUX
diff --git a/hw/virtio.h b/hw/virtio.h
index 69e6bb1..0fd0bb0 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -197,7 +197,8 @@ void virtio_bind_device(VirtIODevice *vdev, const VirtIOBindings *binding,
void *opaque);
/* Base devices. */
-VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf);
+VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf,
+ char **serial);
struct virtio_net_conf;
VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
struct virtio_net_conf *net);
--
1.7.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [PATCH 5/8] block: drive_init(): Simplify interface type setting
2011-07-06 14:21 [Qemu-devel] [PULL 0/8] Block patches Kevin Wolf
` (3 preceding siblings ...)
2011-07-06 14:21 ` [Qemu-devel] [PATCH 4/8] virtio-blk: Turn drive serial into a qdev property Kevin Wolf
@ 2011-07-06 14:21 ` Kevin Wolf
2011-07-06 14:21 ` [Qemu-devel] [PATCH 6/8] block: drive_init(): Improve CHS setting error message Kevin Wolf
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2011-07-06 14:21 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 12 ++++--------
1 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 7d579d6..470be71 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -240,14 +240,6 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
int ret;
translation = BIOS_ATA_TRANSLATION_AUTO;
-
- if (default_to_scsi) {
- type = IF_SCSI;
- pstrcpy(devname, sizeof(devname), "scsi");
- } else {
- type = IF_IDE;
- pstrcpy(devname, sizeof(devname), "ide");
- }
media = MEDIA_DISK;
/* extract parameters */
@@ -273,7 +265,11 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
error_report("unsupported bus type '%s'", buf);
return NULL;
}
+ } else {
+ type = default_to_scsi ? IF_SCSI : IF_IDE;
+ pstrcpy(devname, sizeof(devname), if_name[type]);
}
+
max_devs = if_max_devs[type];
if (cyls || heads || secs) {
--
1.7.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [PATCH 6/8] block: drive_init(): Improve CHS setting error message
2011-07-06 14:21 [Qemu-devel] [PULL 0/8] Block patches Kevin Wolf
` (4 preceding siblings ...)
2011-07-06 14:21 ` [Qemu-devel] [PATCH 5/8] block: drive_init(): Simplify interface type setting Kevin Wolf
@ 2011-07-06 14:21 ` Kevin Wolf
2011-07-06 14:21 ` [Qemu-devel] [PATCH 7/8] ide: Ignore reads during PIO in and writes during PIO out Kevin Wolf
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2011-07-06 14:21 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Luiz Capitulino <lcapitulino@redhat.com>
The current message doesn't clearly communicate the error cause.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 470be71..a97a801 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -310,7 +310,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
media = MEDIA_DISK;
} else if (!strcmp(buf, "cdrom")) {
if (cyls || secs || heads) {
- error_report("'%s' invalid physical CHS format", buf);
+ error_report("CHS can't be set with media=%s", buf);
return NULL;
}
media = MEDIA_CDROM;
--
1.7.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [PATCH 7/8] ide: Ignore reads during PIO in and writes during PIO out
2011-07-06 14:21 [Qemu-devel] [PULL 0/8] Block patches Kevin Wolf
` (5 preceding siblings ...)
2011-07-06 14:21 ` [Qemu-devel] [PATCH 6/8] block: drive_init(): Improve CHS setting error message Kevin Wolf
@ 2011-07-06 14:21 ` Kevin Wolf
2011-07-06 14:21 ` [Qemu-devel] [PATCH 8/8] ide: Initialise buffers with zeros Kevin Wolf
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2011-07-06 14:21 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
This fixes https://bugs.launchpad.net/qemu/+bug/786209:
When the DRQ_STAT bit is set, the IDE core permits both data reads
and data writes, regardless of whether the current transfer was
initiated as a read or write.
This potentially leaks uninitialized host memory into the guest,
if, before doing anything else to an IDE device, the guest begins a
write transaction (e.g. WIN_WRITE), but then *reads* from the IO
port instead of writing to it.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
hw/ide/core.c | 44 ++++++++++++++++++++++++++++++++++++--------
1 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index ca17a43..a29ae9f 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -56,6 +56,7 @@ static const int smart_attributes[][12] = {
};
static int ide_handle_rw_error(IDEState *s, int error, int op);
+static void ide_dummy_transfer_stop(IDEState *s);
static void padstr(char *str, const char *src, int len)
{
@@ -1532,15 +1533,36 @@ void ide_cmd_write(void *opaque, uint32_t addr, uint32_t val)
bus->cmd = val;
}
+/*
+ * Returns true if the running PIO transfer is a PIO out (i.e. data is
+ * transferred from the device to the guest), false if it's a PIO in
+ */
+static bool ide_is_pio_out(IDEState *s)
+{
+ if (s->end_transfer_func == ide_sector_write ||
+ s->end_transfer_func == ide_atapi_cmd) {
+ return false;
+ } else if (s->end_transfer_func == ide_sector_read ||
+ s->end_transfer_func == ide_transfer_stop ||
+ s->end_transfer_func == ide_atapi_cmd_reply_end ||
+ s->end_transfer_func == ide_dummy_transfer_stop) {
+ return true;
+ }
+
+ abort();
+}
+
void ide_data_writew(void *opaque, uint32_t addr, uint32_t val)
{
IDEBus *bus = opaque;
IDEState *s = idebus_active_if(bus);
uint8_t *p;
- /* PIO data access allowed only when DRQ bit is set */
- if (!(s->status & DRQ_STAT))
+ /* PIO data access allowed only when DRQ bit is set. The result of a write
+ * during PIO out is indeterminate, just ignore it. */
+ if (!(s->status & DRQ_STAT) || ide_is_pio_out(s)) {
return;
+ }
p = s->data_ptr;
*(uint16_t *)p = le16_to_cpu(val);
@@ -1557,9 +1579,11 @@ uint32_t ide_data_readw(void *opaque, uint32_t addr)
uint8_t *p;
int ret;
- /* PIO data access allowed only when DRQ bit is set */
- if (!(s->status & DRQ_STAT))
+ /* PIO data access allowed only when DRQ bit is set. The result of a read
+ * during PIO in is indeterminate, return 0 and don't move forward. */
+ if (!(s->status & DRQ_STAT) || !ide_is_pio_out(s)) {
return 0;
+ }
p = s->data_ptr;
ret = cpu_to_le16(*(uint16_t *)p);
@@ -1576,9 +1600,11 @@ void ide_data_writel(void *opaque, uint32_t addr, uint32_t val)
IDEState *s = idebus_active_if(bus);
uint8_t *p;
- /* PIO data access allowed only when DRQ bit is set */
- if (!(s->status & DRQ_STAT))
+ /* PIO data access allowed only when DRQ bit is set. The result of a write
+ * during PIO out is indeterminate, just ignore it. */
+ if (!(s->status & DRQ_STAT) || ide_is_pio_out(s)) {
return;
+ }
p = s->data_ptr;
*(uint32_t *)p = le32_to_cpu(val);
@@ -1595,9 +1621,11 @@ uint32_t ide_data_readl(void *opaque, uint32_t addr)
uint8_t *p;
int ret;
- /* PIO data access allowed only when DRQ bit is set */
- if (!(s->status & DRQ_STAT))
+ /* PIO data access allowed only when DRQ bit is set. The result of a read
+ * during PIO in is indeterminate, return 0 and don't move forward. */
+ if (!(s->status & DRQ_STAT) || !ide_is_pio_out(s)) {
return 0;
+ }
p = s->data_ptr;
ret = cpu_to_le32(*(uint32_t *)p);
--
1.7.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [PATCH 8/8] ide: Initialise buffers with zeros
2011-07-06 14:21 [Qemu-devel] [PULL 0/8] Block patches Kevin Wolf
` (6 preceding siblings ...)
2011-07-06 14:21 ` [Qemu-devel] [PATCH 7/8] ide: Ignore reads during PIO in and writes during PIO out Kevin Wolf
@ 2011-07-06 14:21 ` Kevin Wolf
2011-07-12 9:14 ` [Qemu-devel] [PULL 0/8] Block patches Kevin Wolf
2011-07-12 13:16 ` Anthony Liguori
9 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2011-07-06 14:21 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
Just in case there's still a way how a guest can read out buffers when it's not
supposed to, let's zero the buffers during initialisation so that we don't leak
information to the guest.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
hw/ide/core.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index a29ae9f..d145b19 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1789,9 +1789,13 @@ static void ide_init1(IDEBus *bus, int unit)
s->unit = unit;
s->drive_serial = drive_serial++;
/* we need at least 2k alignment for accessing CDROMs using O_DIRECT */
- s->io_buffer = qemu_memalign(2048, IDE_DMA_BUF_SECTORS*512 + 4);
s->io_buffer_total_len = IDE_DMA_BUF_SECTORS*512 + 4;
+ s->io_buffer = qemu_memalign(2048, s->io_buffer_total_len);
+ memset(s->io_buffer, 0, s->io_buffer_total_len);
+
s->smart_selftest_data = qemu_blockalign(s->bs, 512);
+ memset(s->smart_selftest_data, 0, 512);
+
s->sector_write_timer = qemu_new_timer_ns(vm_clock,
ide_sector_write_timer_cb, s);
}
--
1.7.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PULL 0/8] Block patches
2011-07-06 14:21 [Qemu-devel] [PULL 0/8] Block patches Kevin Wolf
` (7 preceding siblings ...)
2011-07-06 14:21 ` [Qemu-devel] [PATCH 8/8] ide: Initialise buffers with zeros Kevin Wolf
@ 2011-07-12 9:14 ` Kevin Wolf
2011-07-12 13:16 ` Anthony Liguori
9 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2011-07-12 9:14 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel
Am 06.07.2011 16:21, schrieb Kevin Wolf:
> The following changes since commit 9312805d33e8b106bae356d13a8071fb37d75554:
>
> pxa2xx_lcd: add proper rotation support (2011-07-04 22:12:21 +0200)
>
> are available in the git repository at:
> git://repo.or.cz/qemu/kevin.git for-anthony
>
> Federico Simoncelli (1):
> qemu-img: Add cache command line option
>
> Johannes Stezenbach (1):
> block/raw-posix: Linux compat-ioctl warning workaround
>
> Kevin Wolf (3):
> Documentation: Remove outdated host_device note
> ide: Ignore reads during PIO in and writes during PIO out
> ide: Initialise buffers with zeros
>
> Luiz Capitulino (2):
> block: drive_init(): Simplify interface type setting
> block: drive_init(): Improve CHS setting error message
>
> Markus Armbruster (1):
> virtio-blk: Turn drive serial into a qdev property
>
> block/raw-posix.c | 14 +++++++++
> blockdev.c | 14 +++-----
> hw/ide/core.c | 50 +++++++++++++++++++++++++-----
> hw/s390-virtio-bus.c | 4 ++-
> hw/s390-virtio-bus.h | 1 +
> hw/virtio-blk.c | 29 ++++++++++++------
> hw/virtio-blk.h | 2 +
> hw/virtio-pci.c | 4 ++-
> hw/virtio-pci.h | 1 +
> hw/virtio.h | 3 +-
> qemu-img-cmds.hx | 6 ++--
> qemu-img.c | 80 +++++++++++++++++++++++++++++++++++++++++--------
> qemu-img.texi | 6 ----
> 13 files changed, 161 insertions(+), 53 deletions(-)
Ping?
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PULL 0/8] Block patches
2011-07-06 14:21 [Qemu-devel] [PULL 0/8] Block patches Kevin Wolf
` (8 preceding siblings ...)
2011-07-12 9:14 ` [Qemu-devel] [PULL 0/8] Block patches Kevin Wolf
@ 2011-07-12 13:16 ` Anthony Liguori
9 siblings, 0 replies; 11+ messages in thread
From: Anthony Liguori @ 2011-07-12 13:16 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel
On 07/06/2011 09:21 AM, Kevin Wolf wrote:
> The following changes since commit 9312805d33e8b106bae356d13a8071fb37d75554:
>
> pxa2xx_lcd: add proper rotation support (2011-07-04 22:12:21 +0200)
Pulled. Thanks.
Regards,
Anthony Liguori
>
> are available in the git repository at:
> git://repo.or.cz/qemu/kevin.git for-anthony
>
> Federico Simoncelli (1):
> qemu-img: Add cache command line option
>
> Johannes Stezenbach (1):
> block/raw-posix: Linux compat-ioctl warning workaround
>
> Kevin Wolf (3):
> Documentation: Remove outdated host_device note
> ide: Ignore reads during PIO in and writes during PIO out
> ide: Initialise buffers with zeros
>
> Luiz Capitulino (2):
> block: drive_init(): Simplify interface type setting
> block: drive_init(): Improve CHS setting error message
>
> Markus Armbruster (1):
> virtio-blk: Turn drive serial into a qdev property
>
> block/raw-posix.c | 14 +++++++++
> blockdev.c | 14 +++-----
> hw/ide/core.c | 50 +++++++++++++++++++++++++-----
> hw/s390-virtio-bus.c | 4 ++-
> hw/s390-virtio-bus.h | 1 +
> hw/virtio-blk.c | 29 ++++++++++++------
> hw/virtio-blk.h | 2 +
> hw/virtio-pci.c | 4 ++-
> hw/virtio-pci.h | 1 +
> hw/virtio.h | 3 +-
> qemu-img-cmds.hx | 6 ++--
> qemu-img.c | 80 +++++++++++++++++++++++++++++++++++++++++--------
> qemu-img.texi | 6 ----
> 13 files changed, 161 insertions(+), 53 deletions(-)
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread