[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 1/4] block: add the command line support
From: |
Zhi Yong Wu |
Subject: |
Re: [Qemu-devel] [PATCH v5 1/4] block: add the command line support |
Date: |
Wed, 10 Aug 2011 13:20:22 +0800 |
On Tue, Aug 9, 2011 at 8:25 PM, Stefan Hajnoczi <address@hidden> wrote:
> On Tue, Aug 9, 2011 at 5:17 AM, Zhi Yong Wu <address@hidden> wrote:
>> Signed-off-by: Zhi Yong Wu <address@hidden>
>> ---
>> Makefile.objs | 2 +-
>> blockdev.c | 39 +++++++++++++++++++++++++++++++++++++++
>> qemu-config.c | 24 ++++++++++++++++++++++++
>> qemu-option.c | 17 +++++++++++++++++
>> qemu-option.h | 1 +
>> qemu-options.hx | 1 +
>> 6 files changed, 83 insertions(+), 1 deletions(-)
>>
>> diff --git a/Makefile.objs b/Makefile.objs
>> index 9f99ed4..06f2033 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -23,7 +23,7 @@ block-nested-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o
>> dmg.o bochs.o vpc.o vv
>> block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
>> qcow2-cache.o
>> block-nested-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
>> block-nested-y += qed-check.o
>> -block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
>> +block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
>> blk-queue.o
>
> This does not build:
> LINK qemu-ga
> gcc: error: block/blk-queue.o: No such file or directory
>
> This Makefile.objs change should be in the commit that adds blk-queue.c.
>
> Each patch in a series should compile cleanly and can only depend on
> previous patches. This is important so that git-bisect(1) can be
> used, it only works if every commit builds a working program. It also
> makes patch review easier when the patch series builds up logically.
It seems that it will take a bit much time if we strictly stage the
hunks into each corresponding patch.:)
OK, i will.
>
>> block-nested-$(CONFIG_WIN32) += raw-win32.o
>> block-nested-$(CONFIG_POSIX) += raw-posix.o
>> block-nested-$(CONFIG_CURL) += curl.o
>> diff --git a/blockdev.c b/blockdev.c
>> index c263663..9c78548 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -238,6 +238,10 @@ DriveInfo *drive_init(QemuOpts *opts, int
>> default_to_scsi)
>> int on_read_error, on_write_error;
>> const char *devaddr;
>> DriveInfo *dinfo;
>> + BlockIOLimit io_limits;
>
> This structure is not undefined at this point in the patch series.
The hunk contained this structure will be staged in this patch.
>
>> + bool iol_flag = false;
>> + const char *iol_opts[7] = {"bps", "bps_rd", "bps_wr",
>> + "iops", "iops_rd", "iops_wr"};
>> int is_extboot = 0;
>> int snapshot = 0;
>> int ret;
>> @@ -372,6 +376,36 @@ DriveInfo *drive_init(QemuOpts *opts, int
>> default_to_scsi)
>> return NULL;
>> }
>>
>> + /* disk io throttling */
>> + iol_flag = qemu_opt_io_limits_enable_flag(opts, iol_opts);
>> + if (iol_flag) {
>> + memset(&io_limits, 0, sizeof(BlockIOLimit));
>> +
>> + io_limits.bps[BLOCK_IO_LIMIT_TOTAL] =
>> + qemu_opt_get_number(opts, "bps", 0);
>> + io_limits.bps[BLOCK_IO_LIMIT_READ] =
>> + qemu_opt_get_number(opts, "bps_rd", 0);
>> + io_limits.bps[BLOCK_IO_LIMIT_WRITE] =
>> + qemu_opt_get_number(opts, "bps_wr", 0);
>> + io_limits.iops[BLOCK_IO_LIMIT_TOTAL] =
>> + qemu_opt_get_number(opts, "iops", 0);
>> + io_limits.iops[BLOCK_IO_LIMIT_READ] =
>> + qemu_opt_get_number(opts, "iops_rd", 0);
>> + io_limits.iops[BLOCK_IO_LIMIT_WRITE] =
>> + qemu_opt_get_number(opts, "iops_wr", 0);
>> +
>> + if (((io_limits.bps[BLOCK_IO_LIMIT_TOTAL] != 0)
>> + && ((io_limits.bps[BLOCK_IO_LIMIT_READ] != 0)
>> + || (io_limits.bps[BLOCK_IO_LIMIT_WRITE] != 0)))
>> + || ((io_limits.iops[BLOCK_IO_LIMIT_TOTAL] != 0)
>> + && ((io_limits.iops[BLOCK_IO_LIMIT_READ] != 0)
>> + || (io_limits.iops[BLOCK_IO_LIMIT_WRITE] != 0)))) {
>> + error_report("bps(iops) and bps_rd/bps_wr(iops_rd/iops_wr) \
>> + cannot be used at the same time");
>> + return NULL;
>> + }
>> + }
>> +
>> on_write_error = BLOCK_ERR_STOP_ENOSPC;
>> if ((buf = qemu_opt_get(opts, "werror")) != NULL) {
>> if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO && type
>> != IF_NONE) {
>> @@ -483,6 +517,11 @@ DriveInfo *drive_init(QemuOpts *opts, int
>> default_to_scsi)
>>
>> bdrv_set_on_error(dinfo->bdrv, on_read_error, on_write_error);
>>
>> + /* disk I/O throttling */
>> + if (iol_flag) {
>> + bdrv_set_io_limits(dinfo->bdrv, &io_limits);
>> + }
>
> iol_flag and qemu_opt_io_limits_enable_flag() are not necessary. If
> no limits were set then all fields will be 0 (unlimited).
Are they not necessary here? why? qemu_opt_io_limits_enable_flag is
used to determine if io_limits is enabled.
If yes, iol_flag will be set to ONE. So i think that they are necessay here.
>
> Stefan
>
--
Regards,
Zhi Yong Wu
[Qemu-devel] [PATCH v5 2/4] block: add the block queue support, Zhi Yong Wu, 2011/08/09