block: wr_highest_sector blockstat
cover block: wr_highest_sector blockstat
This is the minimal high watermark implementation that is needed to allow clients to poll the value. I hope everyone can live with this solution now.
The second patch of this series is optional. It breaks clients that consider the user monitor a stable API. They should be using QMP, but I'm not sure if we want to break compatibility on the user monitor today.
Kevin Wolf (2):
block: Add wr_highest_sector blockstat
block: Extend info blockstats monitor command
block.c | 103 ++++++++++++++++++++++++++++++++++++++++++++-------------- block_int.h | 1 + qdict.c | 2 +- 3 files changed, 80 insertions(+), 26 deletions(-)
PATCH 1/2 block: Add wr_highest_sector blockstat
This adds the wr_highest_sector blockstat which implements what is generally known as the high watermark. It is the highest offset of a sector written to the respective BlockDriverState since it has been opened.
The query-blockstat QMP command is extended to add this value to the result, and also to add the statistics of the underlying protocol in a new "parent" field. Note that to get the "high watermark" of a qcow2 image, you need to look into the wr_highest_sector field of the parent (which can be a file, a host_device, ...). The wr_highest_sector of the qcow2 BlockDriverState itself is the highest offset on the _virtual_ disk that the guest has written to.
--- block.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++----------- block_int.h | 1 + 2 files changed, 62 insertions(+), 15 deletions(-)
block.c +61 -15
@@ -880,6 +880,10 @@ int bdrv_write(BlockDriverState *bs, int64_t sector_num,set_dirty_bitmap (bs, sector_num, nb_sectors,1 ); } +if (bs->wr_highest_sector < sector_num + nb_sectors -1 ) { + bs->wr_highest_sector = sector_num + nb_sectors -1 ; + } +return drv->bdrv_write (bs, sector_num, buf, nb_sectors); }
@@ -1523,6 +1527,10 @@ static void bdrv_stats_iter(QObject *data, void *opaque)qdict_get_int (qdict,"wr_bytes" ),qdict_get_int (qdict,"rd_operations" ), qdict_get_int(qdict, "wr_operations")); +if (qdict_haskey (qdict,"parent" )) { +QObject *parent =qdict_get (qdict,"parent" ); +bdrv_stats_iter (parent, mon); + } }void bdrv_stats_print (Monitor *mon,const QObject *data)
@@ -1530,6 +1538,34 @@ void bdrv_stats_print(Monitor *mon, const QObject *data)qlist_iter (qobject_to_qlist (data), bdrv_stats_iter, mon); } +static QObject *bdrv_info_stats_bs (BlockDriverState *bs) +{ +QObject *res; +QObject *parent =NULL ; + +if (bs->file) { + parent =bdrv_info_stats_bs (bs->file); + } + + res =qobject_from_jsonf ("{ 'device': %s, 'stats': {" +"'rd_bytes': %" PRId64"," +"'wr_bytes': %" PRId64"," +"'rd_operations': %" PRId64"," +"'wr_operations': %" PRId64"," +"'wr_highest_offset': %" PRId64 +"} }" , + bs->device_name, + bs->rd_bytes, bs->wr_bytes, + bs->rd_ops, bs->wr_ops, + bs->wr_highest_sector *512 ); +if (parent) { +QDict *dict =qobject_to_qdict (res); +qdict_put_obj (dict,"parent" , parent); + } + +return res; +} +/** * bdrv_info_stats(): show block device statistics *
@@ -1544,19 +1580,34 @@ void bdrv_stats_print(Monitor *mon, const QObject *data)* - "wr_bytes": bytes written * - "rd_operations": read operations * - "wr_operations": write operations -* +* - "wr_highest_offset": Highest offset of a sector written since the +* BlockDriverState has been opened +* - "parent": Contains recursively the statistics of the underlying +* protocol (e.g. the host file for a qcow2 image). If there is no +* underlying protocol, this field is omitted. +* * Example: * * [ { "device": "ide0-hd0", * "stats": { "rd_bytes": 512, * "wr_bytes": 0, * "rd_operations": 1, -* "wr_operations": 0 } }, +* "wr_operations": 0, +* "wr_highest_offset": 0, +* "parent": { +* "stats": { "rd_bytes": 1024, +* "wr_bytes": 0, +* "rd_operations": 2, +* "wr_operations": 0, +* "wr_highest_offset": 0, +* } +* } } }, * { "device": "ide1-cd0", * "stats": { "rd_bytes": 0, * "wr_bytes": 0, * "rd_operations": 0, -* "wr_operations": 0 } } ] +* "wr_operations": 0, +* "wr_highest_offset": 0 } }, */ void bdrv_info_stats (Monitor *mon,QObject **ret_data) {
@@ -1567,15 +1618,7 @@ void bdrv_info_stats(Monitor *mon, QObject **ret_data) devices =qlist_new (); QTAILQ_FOREACH(bs, &bdrv_states, list) { - obj =qobject_from_jsonf ("{ 'device': %s, 'stats': {" -"'rd_bytes': %" PRId64"," -"'wr_bytes': %" PRId64"," -"'rd_operations': %" PRId64"," -"'wr_operations': %" PRId64 -"} }" , - bs->device_name, - bs->rd_bytes, bs->wr_bytes, - bs->rd_ops, bs->wr_ops); + obj =bdrv_info_stats_bs (bs);qlist_append_obj (devices, obj); }
@@ -1834,9 +1877,12 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num, cb, opaque);if (ret) { -/* Update stats even though technically transfer has not happened. */ - bs->wr_bytes += (unsigned ) nb_sectors *BDRV_SECTOR_SIZE ; - bs->wr_ops ++; +/* Update stats even though technically transfer has not happened. */ + bs->wr_bytes += (unsigned ) nb_sectors *BDRV_SECTOR_SIZE ; + bs->wr_ops ++; +if (bs->wr_highest_sector < sector_num + nb_sectors -1 ) { + bs->wr_highest_sector = sector_num + nb_sectors -1 ; + } }return ret;
block_int.h +1 -0
@@ -167,6 +167,7 @@ struct BlockDriverState {uint64_t wr_bytes;uint64_t rd_ops;uint64_t wr_ops; +uint64_t wr_highest_sector;/* Whether the disk can expand beyond total_sectors */ int growable;
Signature · 1 line
1.6.6.1
[Qemu-devel] Re: [PATCH 1/2] block: Add wr_highest_sector blockstat
Quoted 11 lines · Kevin Wolf
This adds the
wr_highest_sectorblockstat which implements what is generally known as the high watermark. It is the highest offset of a sector written to the respective BlockDriverState since it has been opened.The query-blockstat QMP command is extended to add this value to the result, and also to add the statistics of the underlying protocol in a new "parent" field. Note that to get the "high watermark" of a qcow2 image, you need to look into the
wr_highest_sectorfield of the parent (which can be a file, ahost_device, ...). Thewr_highest_sectorof the qcow2 BlockDriverState itself is the highest offset on the _virtual_ disk that the guest has written to.
Is that the desirable behavior?
When dealing with layered formats, would it be useful to print out stats for each of the layers?
Regards,
Anthony Liguori
Quoted 151 lines
Signed-off-by Kevin Wolf<[email protected]>--- block.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++----------- block_int.h | 1 + 2 files changed, 62 insertions(+), 15 deletions(-)block.c +61
-15@@ -880,6 +880,10 @@ int bdrv_write(BlockDriverState *bs, int64_t sector_num,set_dirty_bitmap (bs, sector_num, nb_sectors,1 ); } +if (bs->wr_highest_sector< sector_num + nb_sectors -1 ) { + bs->wr_highest_sector = sector_num + nb_sectors -1 ; + } +return drv->bdrv_write (bs, sector_num, buf, nb_sectors); }@@ -1523,6 +1527,10 @@ static void bdrv_stats_iter(QObject *data, void *opaque)qdict_get_int (qdict,"wr_bytes" ),qdict_get_int (qdict,"rd_operations" ), qdict_get_int(qdict, "wr_operations")); +if (qdict_haskey (qdict,"parent" )) { +QObject *parent =qdict_get (qdict,"parent" ); +bdrv_stats_iter (parent, mon); + } }void bdrv_stats_print (Monitor *mon,const QObject *data)@@ -1530,6 +1538,34 @@ void bdrv_stats_print(Monitor *mon, const QObject *data)qlist_iter (qobject_to_qlist (data), bdrv_stats_iter, mon); } +static QObject *bdrv_info_stats_bs (BlockDriverState *bs) +{ +QObject *res; +QObject *parent =NULL ; + +if (bs->file) { + parent =bdrv_info_stats_bs (bs->file); + } + + res =qobject_from_jsonf ("{ 'device': %s, 'stats': {" +"'rd_bytes': %" PRId64"," +"'wr_bytes': %" PRId64"," +"'rd_operations': %" PRId64"," +"'wr_operations': %" PRId64"," +"'wr_highest_offset': %" PRId64 +"} }" , + bs->device_name, + bs->rd_bytes, bs->wr_bytes, + bs->rd_ops, bs->wr_ops, + bs->wr_highest_sector *512 ); +if (parent) { +QDict *dict =qobject_to_qdict (res); +qdict_put_obj (dict,"parent" , parent); + } + +return res; +} +/** * bdrv_info_stats(): show block device statistics * @@ -1544,19 +1580,34 @@ void bdrv_stats_print(Monitor *mon, const QObject *data)* - "wr_bytes": bytes written * - "rd_operations": read operations * - "wr_operations": write operations -* +* - "wr_highest_offset": Highest offset of a sector written since the +* BlockDriverState has been opened +* - "parent": Contains recursively the statistics of the underlying +* protocol (e.g. the host file for a qcow2 image). If there is no +* underlying protocol, this field is omitted. +* * Example: * * [ { "device": "ide0-hd0", * "stats": { "rd_bytes": 512, * "wr_bytes": 0, * "rd_operations": 1, -* "wr_operations": 0 } }, +* "wr_operations": 0, +* "wr_highest_offset": 0, +* "parent": { +* "stats": { "rd_bytes": 1024, +* "wr_bytes": 0, +* "rd_operations": 2, +* "wr_operations": 0, +* "wr_highest_offset": 0, +* } +* } } }, * { "device": "ide1-cd0", * "stats": { "rd_bytes": 0, * "wr_bytes": 0, * "rd_operations": 0, -* "wr_operations": 0 } } ] +* "wr_operations": 0, +* "wr_highest_offset": 0 } }, */ void bdrv_info_stats (Monitor *mon,QObject **ret_data) {@@ -1567,15 +1618,7 @@ void bdrv_info_stats(Monitor *mon, QObject **ret_data) devices =qlist_new (); QTAILQ_FOREACH(bs,&bdrv_states, list) { - obj =qobject_from_jsonf ("{ 'device': %s, 'stats': {" -"'rd_bytes': %" PRId64"," -"'wr_bytes': %" PRId64"," -"'rd_operations': %" PRId64"," -"'wr_operations': %" PRId64 -"} }" , - bs->device_name, - bs->rd_bytes, bs->wr_bytes, - bs->rd_ops, bs->wr_ops); + obj =bdrv_info_stats_bs (bs);qlist_append_obj (devices, obj); }@@ -1834,9 +1877,12 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num, cb, opaque);if (ret) { -/* Update stats even though technically transfer has not happened. */ - bs->wr_bytes += (unsigned ) nb_sectors *BDRV_SECTOR_SIZE ; - bs->wr_ops ++; +/* Update stats even though technically transfer has not happened. */ + bs->wr_bytes += (unsigned ) nb_sectors *BDRV_SECTOR_SIZE ; + bs->wr_ops ++; +if (bs->wr_highest_sector< sector_num + nb_sectors -1 ) { + bs->wr_highest_sector = sector_num + nb_sectors -1 ; + } }return ret;block_int.h +1
-0@@ -167,6 +167,7 @@ struct BlockDriverState {uint64_t wr_bytes;uint64_t rd_ops;uint64_t wr_ops; +uint64_t wr_highest_sector;/* Whether the disk can expand beyond total_sectors */ int growable;
[Qemu-devel] Re: [PATCH 1/2] block: Add wr_highest_sector blockstat
In reply to Kevin WolfQuoted 4 lines · Kevin Wolf
This adds the
wr_highest_sectorblockstat which implements what is generally known as the high watermark. It is the highest offset of a sector written to the respective BlockDriverState since it has been opened.
Quoted 9 lines
The query-blockstat QMP command is extended to add this value to the result, and also to add the statistics of the underlying protocol in a new "parent" field. Note that to get the "high watermark" of a qcow2 image, you need to look into the
wr_highest_sectorfield of the parent (which can be a file, ahost_device, ...). Thewr_highest_sectorof the qcow2 BlockDriverState itself is the highest offset on the _virtual_ disk that the guest has written to.Signed-off-by Kevin Wolf<[email protected]>
I see, you did print out stats for each layer.
I don't think we should take 2/2. I don't mind QMP having more features than the user monitor.
Regards,
Anthony Liguori
Quoted 150 lines
--- block.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++----------- block_int.h | 1 + 2 files changed, 62 insertions(+), 15 deletions(-)block.c +61
-15@@ -880,6 +880,10 @@ int bdrv_write(BlockDriverState *bs, int64_t sector_num,set_dirty_bitmap (bs, sector_num, nb_sectors,1 ); } +if (bs->wr_highest_sector< sector_num + nb_sectors -1 ) { + bs->wr_highest_sector = sector_num + nb_sectors -1 ; + } +return drv->bdrv_write (bs, sector_num, buf, nb_sectors); }@@ -1523,6 +1527,10 @@ static void bdrv_stats_iter(QObject *data, void *opaque)qdict_get_int (qdict,"wr_bytes" ),qdict_get_int (qdict,"rd_operations" ), qdict_get_int(qdict, "wr_operations")); +if (qdict_haskey (qdict,"parent" )) { +QObject *parent =qdict_get (qdict,"parent" ); +bdrv_stats_iter (parent, mon); + } }void bdrv_stats_print (Monitor *mon,const QObject *data)@@ -1530,6 +1538,34 @@ void bdrv_stats_print(Monitor *mon, const QObject *data)qlist_iter (qobject_to_qlist (data), bdrv_stats_iter, mon); } +static QObject *bdrv_info_stats_bs (BlockDriverState *bs) +{ +QObject *res; +QObject *parent =NULL ; + +if (bs->file) { + parent =bdrv_info_stats_bs (bs->file); + } + + res =qobject_from_jsonf ("{ 'device': %s, 'stats': {" +"'rd_bytes': %" PRId64"," +"'wr_bytes': %" PRId64"," +"'rd_operations': %" PRId64"," +"'wr_operations': %" PRId64"," +"'wr_highest_offset': %" PRId64 +"} }" , + bs->device_name, + bs->rd_bytes, bs->wr_bytes, + bs->rd_ops, bs->wr_ops, + bs->wr_highest_sector *512 ); +if (parent) { +QDict *dict =qobject_to_qdict (res); +qdict_put_obj (dict,"parent" , parent); + } + +return res; +} +/** * bdrv_info_stats(): show block device statistics * @@ -1544,19 +1580,34 @@ void bdrv_stats_print(Monitor *mon, const QObject *data)* - "wr_bytes": bytes written * - "rd_operations": read operations * - "wr_operations": write operations -* +* - "wr_highest_offset": Highest offset of a sector written since the +* BlockDriverState has been opened +* - "parent": Contains recursively the statistics of the underlying +* protocol (e.g. the host file for a qcow2 image). If there is no +* underlying protocol, this field is omitted. +* * Example: * * [ { "device": "ide0-hd0", * "stats": { "rd_bytes": 512, * "wr_bytes": 0, * "rd_operations": 1, -* "wr_operations": 0 } }, +* "wr_operations": 0, +* "wr_highest_offset": 0, +* "parent": { +* "stats": { "rd_bytes": 1024, +* "wr_bytes": 0, +* "rd_operations": 2, +* "wr_operations": 0, +* "wr_highest_offset": 0, +* } +* } } }, * { "device": "ide1-cd0", * "stats": { "rd_bytes": 0, * "wr_bytes": 0, * "rd_operations": 0, -* "wr_operations": 0 } } ] +* "wr_operations": 0, +* "wr_highest_offset": 0 } }, */ void bdrv_info_stats (Monitor *mon,QObject **ret_data) {@@ -1567,15 +1618,7 @@ void bdrv_info_stats(Monitor *mon, QObject **ret_data) devices =qlist_new (); QTAILQ_FOREACH(bs,&bdrv_states, list) { - obj =qobject_from_jsonf ("{ 'device': %s, 'stats': {" -"'rd_bytes': %" PRId64"," -"'wr_bytes': %" PRId64"," -"'rd_operations': %" PRId64"," -"'wr_operations': %" PRId64 -"} }" , - bs->device_name, - bs->rd_bytes, bs->wr_bytes, - bs->rd_ops, bs->wr_ops); + obj =bdrv_info_stats_bs (bs);qlist_append_obj (devices, obj); }@@ -1834,9 +1877,12 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num, cb, opaque);if (ret) { -/* Update stats even though technically transfer has not happened. */ - bs->wr_bytes += (unsigned ) nb_sectors *BDRV_SECTOR_SIZE ; - bs->wr_ops ++; +/* Update stats even though technically transfer has not happened. */ + bs->wr_bytes += (unsigned ) nb_sectors *BDRV_SECTOR_SIZE ; + bs->wr_ops ++; +if (bs->wr_highest_sector< sector_num + nb_sectors -1 ) { + bs->wr_highest_sector = sector_num + nb_sectors -1 ; + } }return ret;block_int.h +1
-0@@ -167,6 +167,7 @@ struct BlockDriverState {uint64_t wr_bytes;uint64_t rd_ops;uint64_t wr_ops; +uint64_t wr_highest_sector;/* Whether the disk can expand beyond total_sectors */ int growable;
[Qemu-devel] Re: [PATCH 1/2] block: Add wr_highest_sector blockstat
Quoted 12 lines · On Wed, 28 Apr 2010 11:01:12 -0500 Anthony Liguori <[email protected]>…
Quoted 11 lines · Kevin Wolf
This adds the
wr_highest_sectorblockstat which implements what is generally known as the high watermark. It is the highest offset of a sector written to the respective BlockDriverState since it has been opened.The query-blockstat QMP command is extended to add this value to the result, and also to add the statistics of the underlying protocol in a new "parent" field. Note that to get the "high watermark" of a qcow2 image, you need to look into the
wr_highest_sectorfield of the parent (which can be a file, ahost_device, ...). Thewr_highest_sectorof the qcow2 BlockDriverState itself is the highest offset on the _virtual_ disk that the guest has written to.
Quoted 7 lines
Quoted 2 lines
Signed-off-by Kevin Wolf<[email protected]>I see, you did print out stats for each layer.
I don't think we should take 2/2. I don't mind QMP having more features than the user monitor.
I don't either, but Kevin has said to me that this information is also good
for the user Monitor.
The real question here is whether or not we're going to stop supporting
stability for the user Monitor and if so, when we'll break it.
An arguable reasonable policy would be to try to maintain stability for
existing commands. In this specific case, 'info blockstats' is used by libvirt afaik. So breaking it would mean that older libvirt versions won't be able to talk to newer qemu (taking libvirt just as real known example).
[Qemu-devel] Re: [PATCH 1/2] block: Add wr_highest_sector blockstat
Quoted 28 lines · Luiz Capitulino
Quoted 13 lines · On Wed, 28 Apr 2010 11:01:12 -0500 Anthony Liguori<[email protected]>…
Quoted 11 lines · Kevin Wolf
This adds the
wr_highest_sectorblockstat which implements what is generally known as the high watermark. It is the highest offset of a sector written to the respective BlockDriverState since it has been opened.The query-blockstat QMP command is extended to add this value to the result, and also to add the statistics of the underlying protocol in a new "parent" field. Note that to get the "high watermark" of a qcow2 image, you need to look into the
wr_highest_sectorfield of the parent (which can be a file, ahost_device, ...). Thewr_highest_sectorof the qcow2 BlockDriverState itself is the highest offset on the _virtual_ disk that the guest has written to.Quoted 8 lines
Quoted 3 lines
Signed-off-by Kevin Wolf<[email protected]>I see, you did print out stats for each layer.
I don't think we should take 2/2. I don't mind QMP having more features than the user monitor.
I don't either, but Kevin has said to me that this information is also good
for the user Monitor.
Quoted 8 lines
The real question here is whether or not we're going to stop supporting
stability for the user Monitor and if so, when we'll break it.An arguable reasonable policy would be to try to maintain stability for
existing commands. In this specific case, 'info blockstats' is used by libvirt afaik. So breaking it would mean that older libvirt versions won't be able to talk to newer qemu (taking libvirt just as real known example).
I think we should try our best to maintain compatibility. In this case, this change would break any non-QMP version of libvirt so it would be pretty painful for users. That's why I'm inclined to not take.
It would be reasonable to add a new info command for the user monitor if the functionality is desirable.
Regards,
Anthony Liguori
[Qemu-devel] Re: [PATCH 1/2] block: Add wr_highest_sector blockstat
Quoted 38 lines · On Wed, 28 Apr 2010 12:47:49 -0500 Anthony Liguori <[email protected]>…
Quoted 28 lines · Luiz Capitulino
Quoted 13 lines · On Wed, 28 Apr 2010 11:01:12 -0500 Anthony Liguori<[email protected]>…
Quoted 11 lines · Kevin Wolf
This adds the
wr_highest_sectorblockstat which implements what is generally known as the high watermark. It is the highest offset of a sector written to the respective BlockDriverState since it has been opened.The query-blockstat QMP command is extended to add this value to the result, and also to add the statistics of the underlying protocol in a new "parent" field. Note that to get the "high watermark" of a qcow2 image, you need to look into the
wr_highest_sectorfield of the parent (which can be a file, ahost_device, ...). Thewr_highest_sectorof the qcow2 BlockDriverState itself is the highest offset on the _virtual_ disk that the guest has written to.Quoted 8 lines
Quoted 3 lines
Signed-off-by Kevin Wolf<[email protected]>I see, you did print out stats for each layer.
I don't think we should take 2/2. I don't mind QMP having more features than the user monitor.
I don't either, but Kevin has said to me that this information is also good
for the user Monitor.Quoted 8 lines
The real question here is whether or not we're going to stop supporting
stability for the user Monitor and if so, when we'll break it.An arguable reasonable policy would be to try to maintain stability for
existing commands. In this specific case, 'info blockstats' is used by libvirt afaik. So breaking it would mean that older libvirt versions won't be able to talk to newer qemu (taking libvirt just as real known example).
Quoted 6 lines
I think we should try our best to maintain compatibility. In this case, this change would break any non-QMP version of libvirt so it would be pretty painful for users. That's why I'm inclined to not take.
It would be reasonable to add a new info command for the user monitor if the functionality is desirable.
Seems a good solution to me too.
[Qemu-devel] Re: [PATCH 1/2] block: Add wr_highest_sector blockstat
In reply to Anthony LiguoriQuoted 46 lines · Anthony Liguori
Quoted 36 lines · Luiz Capitulino
Quoted 20 lines · On Wed, 28 Apr 2010 11:01:12 -0500 Anthony Liguori<[email protected]>…
Quoted 18 lines · Kevin Wolf
This adds the
wr_highest_sectorblockstat which implements what is generally
known as the high watermark. It is the highest offset of a sector written to
the respective BlockDriverState since it has been opened.The query-blockstat QMP command is extended to add this value to the result,
and also to add the statistics of the underlying protocol in a new "parent"
field. Note that to get the "high watermark" of a qcow2 image, you need to look
into thewr_highest_sectorfield of the parent (which can be a file, ahost_device, ...). Thewr_highest_sectorof the qcow2 BlockDriverState itself
is the highest offset on the _virtual_ disk that the guest has written to.Quoted 8 lines
Quoted 3 lines
Signed-off-by Kevin Wolf<[email protected]>I see, you did print out stats for each layer.
I don't think we should take 2/2. I don't mind QMP having more features than the user monitor.
I don't either, but Kevin has said to me that this information is also good
for the user Monitor.Quoted 8 lines
The real question here is whether or not we're going to stop supporting
stability for the user Monitor and if so, when we'll break it.An arguable reasonable policy would be to try to maintain stability for
existing commands. In this specific case, 'info blockstats' is used by libvirt afaik. So breaking it would mean that older libvirt versions won't be able to talk to newer qemu (taking libvirt just as real known example).
Quoted 3 lines
I think we should try our best to maintain compatibility. In this case, this change would break any non-QMP version of libvirt so it would be pretty painful for users. That's why I'm inclined to not take.
I agree. There are alot of existing deployed libvirt's out there which all still use the user monitor. libvirt will enable JSON by default once QEMU brings out 0.13, but there'll still be people with older libvirt. We need to avoid breaking the user monitor for at least one or two releaes to allow time for apps to catch up with the switch to JSON. Mark the user monitor deprecated in 0.13 by all means, but keep compatability until either the 0.15 or 0.16 release if at all practical.
Regards,
Daniel
Signature · 4 lines
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
[Qemu-devel] Re: [PATCH 1/2] block: Add wr_highest_sector blockstat
In reply to Anthony LiguoriAm 28.04.2010 18:01, schrieb Anthony Liguori:
Quoted 12 lines · Kevin Wolf
Quoted 11 lines
This adds the
wr_highest_sectorblockstat which implements what is generally known as the high watermark. It is the highest offset of a sector written to the respective BlockDriverState since it has been opened.The query-blockstat QMP command is extended to add this value to the result, and also to add the statistics of the underlying protocol in a new "parent" field. Note that to get the "high watermark" of a qcow2 image, you need to look into the
wr_highest_sectorfield of the parent (which can be a file, ahost_device, ...). Thewr_highest_sectorof the qcow2 BlockDriverState itself is the highest offset on the _virtual_ disk that the guest has written to.
Quoted 7 lines
Quoted 2 lines
Signed-off-by Kevin Wolf<[email protected]>I see, you did print out stats for each layer.
I don't think we should take 2/2. I don't mind QMP having more features than the user monitor.
I agree, just posted it because it felt incomplete without it and I wanted to give you the choice.
The only use case in the user monitor I saw for it would have been qemu development anyway. I can keep the patch in a local branch for this (or just rewrite it when/if I need it - it's small enough).
Kevin
[Qemu-devel] Re: [PATCH 1/2] block: Add wr_highest_sector blockstat
In reply to Kevin WolfQuoted 45 lines · On Wed, 28 Apr 2010 17:56:20 +0200 Kevin Wolf <[email protected]> wrote:
This adds the
wr_highest_sectorblockstat which implements what is generally known as the high watermark. It is the highest offset of a sector written to the respective BlockDriverState since it has been opened.The query-blockstat QMP command is extended to add this value to the result, and also to add the statistics of the underlying protocol in a new "parent" field. Note that to get the "high watermark" of a qcow2 image, you need to look into the
wr_highest_sectorfield of the parent (which can be a file, ahost_device, ...). Thewr_highest_sectorof the qcow2 BlockDriverState itself is the highest offset on the _virtual_ disk that the guest has written to.Signed-off-by Kevin Wolf <[email protected]>--- block.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++----------- block_int.h | 1 + 2 files changed, 62 insertions(+), 15 deletions(-)block.c +8
-0@@ -880,6 +880,10 @@ int bdrv_write(BlockDriverState *bs, int64_t sector_num,set_dirty_bitmap (bs, sector_num, nb_sectors,1 ); } +if (bs->wr_highest_sector < sector_num + nb_sectors -1 ) { + bs->wr_highest_sector = sector_num + nb_sectors -1 ; + } +return drv->bdrv_write (bs, sector_num, buf, nb_sectors); }@@ -1523,6 +1527,10 @@ static void bdrv_stats_iter(QObject *data, void *opaque)qdict_get_int (qdict,"wr_bytes" ),qdict_get_int (qdict,"rd_operations" ), qdict_get_int(qdict, "wr_operations")); +if (qdict_haskey (qdict,"parent" )) { +QObject *parent =qdict_get (qdict,"parent" ); +bdrv_stats_iter (parent, mon); + } }void bdrv_stats_print (Monitor *mon,const QObject *data)@@ -1530,6 +1538,34 @@ void bdrv_stats_print(Monitor *mon, const QObject *data)qlist_iter (qobject_to_qlist (data), bdrv_stats_iter, mon);
block.c void bdrv_stats_print(Monitor *mon, const QObject *data) · line 1548
Quoted 10 lines
block.c +8
-0} +static QObject *bdrv_info_stats_bs (BlockDriverState *bs) +{ +QObject *res; +QObject *parent =NULL ; + +if (bs->file) { + parent =bdrv_info_stats_bs (bs->file); + }
Doesn't build, I guess you meant 'backing_hd'.
Quoted 24 lines
block.c +20
-0+ + res =qobject_from_jsonf ("{ 'device': %s, 'stats': {" +"'rd_bytes': %" PRId64"," +"'wr_bytes': %" PRId64"," +"'rd_operations': %" PRId64"," +"'wr_operations': %" PRId64"," +"'wr_highest_offset': %" PRId64 +"} }" , + bs->device_name, + bs->rd_bytes, bs->wr_bytes, + bs->rd_ops, bs->wr_ops, + bs->wr_highest_sector *512 ); +if (parent) { +QDict *dict =qobject_to_qdict (res); +qdict_put_obj (dict,"parent" , parent); + } + +return res; +} +/** * bdrv_info_stats(): show block device statistics *
block.c void bdrv_stats_print(Monitor *mon, const QObject *data) · line 1587
Quoted 9 lines
block.c +5
-1@@ -1544,19 +1580,34 @@ void bdrv_stats_print(Monitor *mon, const QObject *data)* - "wr_bytes": bytes written * - "rd_operations": read operations * - "wr_operations": write operations -* +* - "wr_highest_offset": Highest offset of a sector written since the +* BlockDriverState has been opened +* - "parent": Contains recursively the statistics of the underlying +* protocol (e.g. the host file for a qcow2 image). If there is no +* underlying protocol, this field is omitted.
Looks like you are not pushing 'protocol', we already have 'format'
though (not sure if they overlap).
Also, 'device' is "" for 'parent'. You should consider not pushing the
key in this case (and noting that it's optional in the doc above).
Quoted 73 lines
block.c +20
-14+* * Example: * * [ { "device": "ide0-hd0", * "stats": { "rd_bytes": 512, * "wr_bytes": 0, * "rd_operations": 1, -* "wr_operations": 0 } }, +* "wr_operations": 0, +* "wr_highest_offset": 0, +* "parent": { +* "stats": { "rd_bytes": 1024, +* "wr_bytes": 0, +* "rd_operations": 2, +* "wr_operations": 0, +* "wr_highest_offset": 0, +* } +* } } }, * { "device": "ide1-cd0", * "stats": { "rd_bytes": 0, * "wr_bytes": 0, * "rd_operations": 0, -* "wr_operations": 0 } } ] +* "wr_operations": 0, +* "wr_highest_offset": 0 } }, */ void bdrv_info_stats (Monitor *mon,QObject **ret_data) {@@ -1567,15 +1618,7 @@ void bdrv_info_stats(Monitor *mon, QObject **ret_data) devices =qlist_new (); QTAILQ_FOREACH(bs, &bdrv_states, list) { - obj =qobject_from_jsonf ("{ 'device': %s, 'stats': {" -"'rd_bytes': %" PRId64"," -"'wr_bytes': %" PRId64"," -"'rd_operations': %" PRId64"," -"'wr_operations': %" PRId64 -"} }" , - bs->device_name, - bs->rd_bytes, bs->wr_bytes, - bs->rd_ops, bs->wr_ops); + obj =bdrv_info_stats_bs (bs);qlist_append_obj (devices, obj); }@@ -1834,9 +1877,12 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num, cb, opaque);if (ret) { -/* Update stats even though technically transfer has not happened. */ - bs->wr_bytes += (unsigned ) nb_sectors *BDRV_SECTOR_SIZE ; - bs->wr_ops ++; +/* Update stats even though technically transfer has not happened. */ + bs->wr_bytes += (unsigned ) nb_sectors *BDRV_SECTOR_SIZE ; + bs->wr_ops ++; +if (bs->wr_highest_sector < sector_num + nb_sectors -1 ) { + bs->wr_highest_sector = sector_num + nb_sectors -1 ; + } }return ret;block_int.h +1
-0@@ -167,6 +167,7 @@ struct BlockDriverState {uint64_t wr_bytes;uint64_t rd_ops;uint64_t wr_ops; +uint64_t wr_highest_sector;/* Whether the disk can expand beyond total_sectors */ int growable;
[Qemu-devel] Re: [PATCH 1/2] block: Add wr_highest_sector blockstat
Am 28.04.2010 19:31, schrieb Luiz Capitulino:
Quoted 50 lines
Quoted 47 lines · On Wed, 28 Apr 2010 17:56:20 +0200 Kevin Wolf <[email protected]> wrote:
This adds the
wr_highest_sectorblockstat which implements what is generally known as the high watermark. It is the highest offset of a sector written to the respective BlockDriverState since it has been opened.The query-blockstat QMP command is extended to add this value to the result, and also to add the statistics of the underlying protocol in a new "parent" field. Note that to get the "high watermark" of a qcow2 image, you need to look into the
wr_highest_sectorfield of the parent (which can be a file, ahost_device, ...). Thewr_highest_sectorof the qcow2 BlockDriverState itself is the highest offset on the _virtual_ disk that the guest has written to.Signed-off-by Kevin Wolf <[email protected]>--- block.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++----------- block_int.h | 1 + 2 files changed, 62 insertions(+), 15 deletions(-)block.c +8
-0@@ -880,6 +880,10 @@ int bdrv_write(BlockDriverState *bs, int64_t sector_num,set_dirty_bitmap (bs, sector_num, nb_sectors,1 ); } +if (bs->wr_highest_sector < sector_num + nb_sectors -1 ) { + bs->wr_highest_sector = sector_num + nb_sectors -1 ; + } +return drv->bdrv_write (bs, sector_num, buf, nb_sectors); }@@ -1523,6 +1527,10 @@ static void bdrv_stats_iter(QObject *data, void *opaque)qdict_get_int (qdict,"wr_bytes" ),qdict_get_int (qdict,"rd_operations" ), qdict_get_int(qdict, "wr_operations")); +if (qdict_haskey (qdict,"parent" )) { +QObject *parent =qdict_get (qdict,"parent" ); +bdrv_stats_iter (parent, mon); + } }void bdrv_stats_print (Monitor *mon,const QObject *data)@@ -1530,6 +1538,34 @@ void bdrv_stats_print(Monitor *mon, const QObject *data)qlist_iter (qobject_to_qlist (data), bdrv_stats_iter, mon); }
Quoted 10 lines
block.c void bdrv_stats_print(Monitor *mon, const QObject *data) · line 1548
Quoted 8 lines
block.c +8
-0+static QObject *bdrv_info_stats_bs (BlockDriverState *bs) +{ +QObject *res; +QObject *parent =NULL ; + +if (bs->file) { + parent =bdrv_info_stats_bs (bs->file); + }Doesn't build, I guess you meant '
backing_hd'.
The patch is against git://repo.or.cz/qemu/kevin.git block, not master (like most block related patches lately).
backing_hd is for qcow base images, so that's a different thing.
Quoted 28 lines
Quoted 28 lines
block.c +20
-1+ + res =qobject_from_jsonf ("{ 'device': %s, 'stats': {" +"'rd_bytes': %" PRId64"," +"'wr_bytes': %" PRId64"," +"'rd_operations': %" PRId64"," +"'wr_operations': %" PRId64"," +"'wr_highest_offset': %" PRId64 +"} }" , + bs->device_name, + bs->rd_bytes, bs->wr_bytes, + bs->rd_ops, bs->wr_ops, + bs->wr_highest_sector *512 ); +if (parent) { +QDict *dict =qobject_to_qdict (res); +qdict_put_obj (dict,"parent" , parent); + } + +return res; +} +/** * bdrv_info_stats(): show block device statistics * @@ -1544,19 +1580,34 @@ void bdrv_stats_print(Monitor *mon, const QObject *data)* - "wr_bytes": bytes written * - "rd_operations": read operations * - "wr_operations": write operations -*
Quoted 8 lines
block.c void bdrv_stats_print(Monitor *mon, const QObject *data) · line 1587
Quoted 5 lines
block.c +5
-0+* - "wr_highest_offset": Highest offset of a sector written since the +* BlockDriverState has been opened +* - "parent": Contains recursively the statistics of the underlying +* protocol (e.g. the host file for a qcow2 image). If there is no +* underlying protocol, this field is omitted. Looks like you are not pushing 'protocol', we already have 'format'
though (not sure if they overlap).
You're talking about a new key 'protocol'? I'm not sure what it should describe, to be honest.
Quoted 2 lines
Also, 'device' is "" for 'parent'. You should consider not pushing the
key in this case (and noting that it's optional in the doc above).
Ok, will change that for v2.
Kevin
[Qemu-devel] Re: [PATCH 1/2] block: Add wr_highest_sector blockstat
Quoted 17 lines · On Thu, 29 Apr 2010 12:00:45 +0200 Kevin Wolf <[email protected]> wrote:
Quoted 8 lines
Quoted 8 lines
/**
*bdrv_info_stats(): show block device statistics *block.c +0
-1@@ -1544,19 +1580,34 @@ void bdrv_stats_print(Monitor *mon, const QObject *data)* - "wr_bytes": bytes written * - "rd_operations": read operations * - "wr_operations": write operations -* Quoted 8 lines
block.c void bdrv_stats_print(Monitor *mon, const QObject *data) · line 1587
Quoted 5 lines
block.c +5
-0+* - "wr_highest_offset": Highest offset of a sector written since the +* BlockDriverState has been opened +* - "parent": Contains recursively the statistics of the underlying +* protocol (e.g. the host file for a qcow2 image). If there is no +* underlying protocol, this field is omitted. Looks like you are not pushing 'protocol', we already have 'format'
though (not sure if they overlap).
Quoted 2 lines
You're talking about a new key 'protocol'? I'm not sure what it should describe, to be honest.
I thought I read somewhere you were going to have a 'protocol' key as well
(in addition to 'wr_highest_offset' and 'parent') but looks like I was just confused.
Sorry for the noise.
PATCH 2/2 block: Extend info blockstats monitor command
In reply to Kevin WolfNow the high watermark and statistics of the underlying images are exposed via QMP, but they are missing in the user monitor. This patch changes the user monitor to provide the same functionality.
Note that it's not possible to maintain compatibility with older clients that try to parse the output of this command. They need to use QMP for a stable API.
The new info blockstats output looks like this (and yes, the example shows that read/write statistics are broken currently, they ignore synchronous I/O):
(qemu) info blockstats
ide0-hd0:
qcow2 rd_bytes=814592 rd_ops=1591
wr_bytes=0 wr_ops=0 wr_highest_offset=0
file rd_bytes=814592 rd_ops=1591
wr_bytes=0 wr_ops=0 wr_highest_offset=0
--- block.c | 35 +++++++++++++++++++++-------------- qdict.c | 2 +- 2 files changed, 22 insertions(+), 15 deletions(-)
block.c +21 -14
@@ -1517,19 +1517,25 @@ static void bdrv_stats_iter(QObject *data, void *opaque) qdict =qobject_to_qdict (data);monitor_printf (mon,"%s:" ,qdict_get_str (qdict,"device" )); - qdict =qobject_to_qdict (qdict_get (qdict,"stats" )); -monitor_printf (mon," rd_bytes=%" PRId64 -" wr_bytes=%" PRId64 -" rd_operations=%" PRId64 -" wr_operations=%" PRId64 -"\n" , -qdict_get_int (qdict,"rd_bytes" ), -qdict_get_int (qdict,"wr_bytes" ), -qdict_get_int (qdict,"rd_operations" ), -qdict_get_int (qdict,"wr_operations" )); -if (qdict_haskey (qdict,"parent" )) { -QObject *parent =qdict_get (qdict,"parent" ); -bdrv_stats_iter (parent, mon); +while (qdict) { +QDict *stats =qobject_to_qdict (qdict_get (qdict,"stats" )); + +monitor_printf (mon,"\n %-12s rd_bytes=%" PRId64 +" rd_ops=%" PRId64 +"\n %-12s wr_bytes=%" PRId64 +" wr_ops=%" PRId64 +" wr_highest_offset=%" PRId64 +"\n" , +qdict_get_str (qdict,"format" ), +qdict_get_int (stats,"rd_bytes" ), +qdict_get_int (stats,"rd_operations" ), +"" , +qdict_get_int (stats,"wr_bytes" ), +qdict_get_int (stats,"wr_operations" ), +qdict_get_int (stats,"wr_highest_offset" )); + + + qdict =qobject_to_qdict (qdict_get (qdict,"parent" )); } }
@@ -1547,7 +1553,7 @@ static QObject* bdrv_info_stats_bs(BlockDriverState *bs) parent =bdrv_info_stats_bs (bs->file); } - res =qobject_from_jsonf ("{ 'device': %s, 'stats': {" + res =qobject_from_jsonf ("{ 'device': %s, 'format': %s, 'stats': {" "'rd_bytes': %" PRId64"," "'wr_bytes': %" PRId64"," "'rd_operations': %" PRId64 ","
@@ -1555,6 +1561,7 @@ static QObject* bdrv_info_stats_bs(BlockDriverState *bs)"'wr_highest_offset': %" PRId64"} }" , bs->device_name, + bs->drv ? bs->drv->format_name :"none" , bs->rd_bytes, bs->wr_bytes, bs->rd_ops, bs->wr_ops, bs->wr_highest_sector * 512);
qdict.c +1 -1
@@ -46,7 +46,7 @@ QDict *qdict_new(void)*/ QDict *qobject_to_qdict (const QObject *obj) { -if (qobject_type (obj) !=QTYPE_QDICT ) +if (!obj ||qobject_type (obj) !=QTYPE_QDICT )return NULL ;return container_of (obj, QDict, base);
Signature · 1 line
1.6.6.1