Re: [Qemu-block] [PATCH v2 3/3] xen-disk: use an IOThread per instance
CC'ing Andreas Färber. Could you please give a quick look below at the
way the iothread object is instantiate and destroyed? I am no object
model expert and would appreaciate a second opinion.
On Wed, 21 Jun 2017, Paul Durrant wrote:
> This patch allocates an IOThread object for each xen_disk instance and
> sets the AIO context appropriately on connect. This allows processing
> of I/O to proceed in parallel.
>
> The patch also adds tracepoints into xen_disk to make it possible to
> follow the state transtions of an instance in the log.
>
> Signed-off-by: Paul Durrant
> ---
> Cc: Stefano Stabellini
> Cc: Anthony Perard
> Cc: Kevin Wolf
> Cc: Max Reitz
>
> v2:
> - explicitly acquire and release AIO context in qemu_aio_complete() and
>blk_bh()
> ---
> hw/block/trace-events | 7 ++
> hw/block/xen_disk.c | 69
> ---
> 2 files changed, 67 insertions(+), 9 deletions(-)
>
> diff --git a/hw/block/trace-events b/hw/block/trace-events
> index 65e83dc258..608b24ba66 100644
> --- a/hw/block/trace-events
> +++ b/hw/block/trace-events
> @@ -10,3 +10,10 @@ virtio_blk_submit_multireq(void *mrb, int start, int
> num_reqs, uint64_t offset,
> # hw/block/hd-geometry.c
> hd_geometry_lchs_guess(void *blk, int cyls, int heads, int secs) "blk %p
> LCHS %d %d %d"
> hd_geometry_guess(void *blk, uint32_t cyls, uint32_t heads, uint32_t secs,
> int trans) "blk %p CHS %u %u %u trans %d"
> +
> +# hw/block/xen_disk.c
> +xen_disk_alloc(char *name) "%s"
> +xen_disk_init(char *name) "%s"
> +xen_disk_connect(char *name) "%s"
> +xen_disk_disconnect(char *name) "%s"
> +xen_disk_free(char *name) "%s"
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 0e6513708e..8548195195 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -27,10 +27,13 @@
> #include "hw/xen/xen_backend.h"
> #include "xen_blkif.h"
> #include "sysemu/blockdev.h"
> +#include "sysemu/iothread.h"
> #include "sysemu/block-backend.h"
> #include "qapi/error.h"
> #include "qapi/qmp/qdict.h"
> #include "qapi/qmp/qstring.h"
> +#include "qom/object_interfaces.h"
> +#include "trace.h"
>
> /* - */
>
> @@ -128,6 +131,9 @@ struct XenBlkDev {
> DriveInfo *dinfo;
> BlockBackend*blk;
> QEMUBH *bh;
> +
> +IOThread*iothread;
> +AioContext *ctx;
> };
>
> /* - */
> @@ -599,9 +605,12 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq);
> static void qemu_aio_complete(void *opaque, int ret)
> {
> struct ioreq *ioreq = opaque;
> +struct XenBlkDev *blkdev = ioreq->blkdev;
> +
> +aio_context_acquire(blkdev->ctx);
I think that Paolo was right that we need a aio_context_acquire here,
however the issue is that with the current code:
blk_handle_requests -> ioreq_runio_qemu_aio -> qemu_aio_complete
leading to aio_context_acquire being called twice on the same lock,
which I don't think is allowed?
I think we need to get rid of the qemu_aio_complete call from
ioreq_runio_qemu_aio, but to do that we need to be careful with the
accounting of aio_inflight (today it's incremented unconditionally at
the beginning of ioreq_runio_qemu_aio, I think we would have to change
that to increment it only if presync).
> if (ret != 0) {
> -xen_pv_printf(&ioreq->blkdev->xendev, 0, "%s I/O error\n",
> +xen_pv_printf(&blkdev->xendev, 0, "%s I/O error\n",
>ioreq->req.operation == BLKIF_OP_READ ? "read" :
> "write");
> ioreq->aio_errors++;
> }
> @@ -610,13 +619,13 @@ static void qemu_aio_complete(void *opaque, int ret)
> if (ioreq->presync) {
> ioreq->presync = 0;
> ioreq_runio_qemu_aio(ioreq);
> -return;
> +goto done;
> }
> if (ioreq->aio_inflight > 0) {
> -return;
> +goto done;
> }
>
> -if (ioreq->blkdev->feature_grant_copy) {
> +if (blkdev->feature_grant_copy) {
> switch (ioreq->req.operation) {
> case BLKIF_OP_READ:
> /* in case of failure ioreq->aio_errors is increased */
> @@ -638,7 +647,7 @@ static void qemu_aio_complete(void *opaque, int ret)
> }
>
> ioreq->status = ioreq->aio_errors ? BLKIF_RSP_ERROR : BLKIF_RSP_OKAY;
> -if (!ioreq->blkdev->feature_grant_copy) {
> +if (!blkdev->feature_grant_copy) {
> ioreq_unmap(ioreq);
> }
> ioreq_finish(ioreq);
> @@ -650,16 +659,19 @@ static void qemu_aio_complete(void *opaque, int ret)
> }
> case BLKIF_OP_READ:
> if (ioreq->status == BLKIF_RSP_OKAY) {
> -block_acct_done(blk_get_stats(ioreq->blkdev->blk), &ioreq->acct);
> +block_acct_done(blk_get_stats(blkdev->blk), &ioreq->acct);
> } else {
> -block_acct_failed(blk_get_stats(ioreq->blkdev->blk),
> &ioreq->acct);
> +
Re: [Qemu-block] [PATCH v2 00/31] qed: Convert to coroutines
Am 16.06.2017 um 19:36 hat Kevin Wolf geschrieben: > The qed block driver is one of the last remaining block drivers that use the > AIO callback style interfaces. This series converts it to the coroutine model > that other drivers are using and removes some AIO functions from the block > layer API afterwards. > > If this isn't compelling enough, the diffstat should speak for itself. > > This series is relatively long, but it consists mostly of mechanical > conversions of one function per patch, so it should be easy to review. > > v2: > - Add coroutine_fn markers [Stefan, Paolo] > - Use bdrv_co_*() instead of bdrv_*() in coroutine_fns > - Use ACB on stack in qed_co_request [Paolo] > - Updated some comments [Paolo] > - Unplug earlier in qed_clear_need_check() [Stefan] > - Removed now unused trace events [Stefan] > - Improved commit message of patch creating qed_aio_write_cow() [Eric] Applied to the block branch. Kevin
Re: [Qemu-block] [PATCH v2 1/4] qemu-img: add --shrink flag for resize
On 22.06.2017 17:49, Kevin Wolf wrote: Am 22.06.2017 um 15:54 hat Pavel Butsykin geschrieben: On 22.06.2017 01:17, Max Reitz wrote: On 2017-06-13 14:16, Pavel Butsykin wrote: The flag as additional precaution of data loss. Perhaps in the future the operation shrink without this flag will be banned, but while we need to maintain compatibility. Signed-off-by: Pavel Butsykin The functional changes look good to me; even though I'd rather have it an error for qcow2 now (even if that means having to check the image format in img_resize(), and being inconsistent because you wouldn't need --shrink for raw, but for qcow2 you would). But, well, I'm not going to stop this series over that. Why shrink for qcow2 image is dangerous, but for raw is not dangerous? I think we should provide the same behavior for all formats. When --shrink option will become necessary, it also should be the same for all image formats. It is dangerous for both, but for raw we can't enforce the flag immediately without a deprecation period. With qcow2 we can (because it is new functionality), so we might as well enforce it from the start. Ah, exactly. I like the offer to print the warning for raw and enforce the flag for other formats. Kevin
Re: [Qemu-block] [Qemu-devel] [PATCH v2 2/4] qapi: Add qobject_is_equal()
Max Reitz writes:
> On 2017-06-21 18:43, Markus Armbruster wrote:
>> Max Reitz writes:
>>
>>> This generic function (along with its implementations for different
>>> types) determines whether two QObjects are equal.
>>>
>>> Signed-off-by: Max Reitz
>>> ---
>>> include/qapi/qmp/qbool.h | 1 +
>>> include/qapi/qmp/qdict.h | 1 +
>>> include/qapi/qmp/qfloat.h | 1 +
>>> include/qapi/qmp/qint.h| 1 +
>>> include/qapi/qmp/qlist.h | 1 +
>>> include/qapi/qmp/qnull.h | 2 ++
>>> include/qapi/qmp/qobject.h | 9 +
>>> include/qapi/qmp/qstring.h | 1 +
>>> qobject/qbool.c| 8
>>> qobject/qdict.c| 28
>>> qobject/qfloat.c | 8
>>> qobject/qint.c | 8
>>> qobject/qlist.c| 30 ++
>>> qobject/qnull.c| 5 +
>>> qobject/qobject.c | 30 ++
>>> qobject/qstring.c | 9 +
>>> 16 files changed, 143 insertions(+)
>>
>> No unit test?
>
> *cough*
>
>>> diff --git a/include/qapi/qmp/qbool.h b/include/qapi/qmp/qbool.h
>>> index a4c..f77ea86 100644
>>> --- a/include/qapi/qmp/qbool.h
>>> +++ b/include/qapi/qmp/qbool.h
>>> @@ -24,6 +24,7 @@ typedef struct QBool {
>>> QBool *qbool_from_bool(bool value);
>>> bool qbool_get_bool(const QBool *qb);
>>> QBool *qobject_to_qbool(const QObject *obj);
>>> +bool qbool_is_equal(const QObject *x, const QObject *y);
>>> void qbool_destroy_obj(QObject *obj);
>>>
>>> #endif /* QBOOL_H */
>>> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
>>> index 188440a..134a526 100644
>>> --- a/include/qapi/qmp/qdict.h
>>> +++ b/include/qapi/qmp/qdict.h
>>> @@ -41,6 +41,7 @@ void qdict_del(QDict *qdict, const char *key);
>>> int qdict_haskey(const QDict *qdict, const char *key);
>>> QObject *qdict_get(const QDict *qdict, const char *key);
>>> QDict *qobject_to_qdict(const QObject *obj);
>>> +bool qdict_is_equal(const QObject *x, const QObject *y);
>>> void qdict_iter(const QDict *qdict,
>>> void (*iter)(const char *key, QObject *obj, void *opaque),
>>> void *opaque);
>>> diff --git a/include/qapi/qmp/qfloat.h b/include/qapi/qmp/qfloat.h
>>> index b5d1583..318e91e 100644
>>> --- a/include/qapi/qmp/qfloat.h
>>> +++ b/include/qapi/qmp/qfloat.h
>>> @@ -24,6 +24,7 @@ typedef struct QFloat {
>>> QFloat *qfloat_from_double(double value);
>>> double qfloat_get_double(const QFloat *qi);
>>> QFloat *qobject_to_qfloat(const QObject *obj);
>>> +bool qfloat_is_equal(const QObject *x, const QObject *y);
>>> void qfloat_destroy_obj(QObject *obj);
>>>
>>> #endif /* QFLOAT_H */
>>> diff --git a/include/qapi/qmp/qint.h b/include/qapi/qmp/qint.h
>>> index 3aaff76..20975da 100644
>>> --- a/include/qapi/qmp/qint.h
>>> +++ b/include/qapi/qmp/qint.h
>>> @@ -23,6 +23,7 @@ typedef struct QInt {
>>> QInt *qint_from_int(int64_t value);
>>> int64_t qint_get_int(const QInt *qi);
>>> QInt *qobject_to_qint(const QObject *obj);
>>> +bool qint_is_equal(const QObject *x, const QObject *y);
>>> void qint_destroy_obj(QObject *obj);
>>>
>>> #endif /* QINT_H */
>>> diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
>>> index 5dc4ed9..4380a5b 100644
>>> --- a/include/qapi/qmp/qlist.h
>>> +++ b/include/qapi/qmp/qlist.h
>>> @@ -57,6 +57,7 @@ QObject *qlist_peek(QList *qlist);
>>> int qlist_empty(const QList *qlist);
>>> size_t qlist_size(const QList *qlist);
>>> QList *qobject_to_qlist(const QObject *obj);
>>> +bool qlist_is_equal(const QObject *x, const QObject *y);
>>> void qlist_destroy_obj(QObject *obj);
>>>
>>> static inline const QListEntry *qlist_first(const QList *qlist)
>>> diff --git a/include/qapi/qmp/qnull.h b/include/qapi/qmp/qnull.h
>>> index 69555ac..9299683 100644
>>> --- a/include/qapi/qmp/qnull.h
>>> +++ b/include/qapi/qmp/qnull.h
>>> @@ -23,4 +23,6 @@ static inline QObject *qnull(void)
>>> return &qnull_;
>>> }
>>>
>>> +bool qnull_is_equal(const QObject *x, const QObject *y);
>>> +
>>> #endif /* QNULL_H */
>>> diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
>>> index ef1d1a9..cac72e3 100644
>>> --- a/include/qapi/qmp/qobject.h
>>> +++ b/include/qapi/qmp/qobject.h
>>> @@ -68,6 +68,15 @@ static inline void qobject_incref(QObject *obj)
>>> }
>>>
>>> /**
>>> + * qobject_is_equal(): Returns whether the two objects are equal.
>>
>> Imperative mood, please: Return whether...
>
> OK, will do here and everywhere else.
>
>>> + *
>>> + * Any of the pointers may be NULL; will return true if both are. Will
>>> always
>>> + * return false if only one is (therefore a QNull object is not considered
>>> equal
>>> + * to a NULL pointer).
>>> + */
>>
>> Humor me: wrap comment lines around column 70, and put two spaces
>> between sentences.
>
> Do I hear "one checkpatch.pl per subsystem"?
Nah, you hear "would you mind doing me a favor if I ask nicely?"
> Yes, I know, t
Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/4] block: qobject_is_equal() in bdrv_reopen_prepare()
Max Reitz writes:
> On 2017-06-21 18:06, Markus Armbruster wrote:
>> Max Reitz writes:
>>
>>> Currently, bdrv_reopen_prepare() assumes that all BDS options are
>>> strings. However, this is not the case if the BDS has been created
>>> through the json: pseudo-protocol or blockdev-add.
>>>
>>> Note that the user-invokable reopen command is an HMP command, so you
>>> can only specify strings there. Therefore, specifying a non-string
>>> option with the "same" value as it was when originally created will now
>>> return an error because the values are supposedly similar (and there is
>>> no way for the user to circumvent this but to just not specify the
>>> option again -- however, this is still strictly better than just
>>> crashing).
>>>
>>> Reviewed-by: Kevin Wolf
>>> Signed-off-by: Max Reitz
>>> ---
>>> block.c | 15 +++
>>> 1 file changed, 3 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/block.c b/block.c
>>> index fa1d06d..23424d5 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -2950,19 +2950,10 @@ int bdrv_reopen_prepare(BDRVReopenState
>>> *reopen_state, BlockReopenQueue *queue,
>>> const QDictEntry *entry = qdict_first(reopen_state->options);
>>>
>>> do {
>>> -QString *new_obj = qobject_to_qstring(entry->value);
>>> -const char *new = qstring_get_str(new_obj);
>>> -/*
>>> - * Caution: while qdict_get_try_str() is fine, getting
>>> - * non-string types would require more care. When
>>> - * bs->options come from -blockdev or blockdev_add, its
>>> - * members are typed according to the QAPI schema, but
>>> - * when they come from -drive, they're all QString.
>>> - */
>>
>> Your commit message makes me suspect this comment still applies in some
>> form. Does it?
>
> The only thing that I can think of that may be applicable is what I
> wrote in the commit message; this fails now:
>
> $ qemu-io -c 'reopen -o size=65536' \
> "json:{'driver':'null-co','size':65536}"
> Cannot change the option 'size'
>
> As I wrote in the commit message, though, I don't think this is too bad.
> First, before, it just crashed, so this definitely is better behavior.
>
> Secondly, I think this is just good for convenience; it allows the user
> to specify an option (to "change" it) even if the block driver doesn't
> support changing it. If it actually has not change, this is supposed to
> be handled gracefully. But sometimes we cannot easily handle it, so...
> We just give an error.
>
> I suspect that at some point we want to fix this by having everything
> correctly typed internally...? Until then, in my opinion, we can just
> not provide this feature.
Correcting the types in a QDict to match the schema is no easier than
converting it to a QAPI generated C type. In my opinion we should work
towards the latter (QAPIfy stuff) instead of the former (dig us ever
deeper into the QObject hole).
> However, I should have probably not just deleted the comment and hoped
> everyone can find the necessary information through git-blame. I should
> leave a comment about this here.
Yes, please!
> Or we do make an effort to provide this test-unchanged functionality for
> every case, in which case we would have to convert either string options
> to the correct type (according to the schema) here (but if that were so
> simple, we could do that centrally in bdrv_open() already, right?) or
The one way we already have to check QObject types against the schema is
the QObject input visitor. So, to get a QObject with correct types, you
can convert to the QAPI-generated C type with the appropriate variant of
the QObject input visitor, then right back with the QObject output
visitor.
But once we have the QAPI-generated C type, we should simply use *that*
instead of dicking around with QDict. In other words: QAPIfy.
"Appropriate variant" means you need to choose between the variant that
expects correct scalar types (appropriate when the QObject comes from
JSON) and the keyval variant, which expects strings (appropriate when
the QObject comes from keyval_parse()).
> convert typed options to strings and compare them -- but since there are
> probably multiple different strings that can mean the same value for
> whatever type the option has, this won't work in every case either.
Converting to string throws away information. Not sure that works.
> So I'm for leaving a comment and leaving this not quite as it should be
> until we have fixed all of the block layer (O:-)). Maybe you have a
> better idea though, which would be great.
Let's go with a comment.
Re: [Qemu-block] [PATCH v2 1/4] qemu-img: add --shrink flag for resize
Am 22.06.2017 um 15:54 hat Pavel Butsykin geschrieben: > On 22.06.2017 01:17, Max Reitz wrote: > >On 2017-06-13 14:16, Pavel Butsykin wrote: > >>The flag as additional precaution of data loss. Perhaps in the future the > >>operation shrink without this flag will be banned, but while we need to > >>maintain compatibility. > >> > >>Signed-off-by: Pavel Butsykin > >The functional changes look good to me; even though I'd rather have it > >an error for qcow2 now (even if that means having to check the image > >format in img_resize(), and being inconsistent because you wouldn't need > >--shrink for raw, but for qcow2 you would). But, well, I'm not going to > >stop this series over that. > > > > Why shrink for qcow2 image is dangerous, but for raw is not dangerous? I > think we should provide the same behavior for all formats. When --shrink > option will become necessary, it also should be the same for all image > formats. It is dangerous for both, but for raw we can't enforce the flag immediately without a deprecation period. With qcow2 we can (because it is new functionality), so we might as well enforce it from the start. Kevin
Re: [Qemu-block] [Qemu-devel] [PATCH v2 1/4] qapi/qnull: Add own header
Max Reitz writes: > On 2017-06-21 18:24, Markus Armbruster wrote: >> Max Reitz writes: >> >>> Reviewed-by: Kevin Wolf >>> Signed-off-by: Max Reitz >>> --- >>> include/qapi/qmp/qnull.h | 26 ++ >>> include/qapi/qmp/qobject.h | 8 >>> include/qapi/qmp/types.h | 1 + >>> qobject/qnull.c| 1 + >>> target/i386/cpu.c | 6 +- >>> tests/check-qnull.c| 2 +- >>> 6 files changed, 30 insertions(+), 14 deletions(-) >>> create mode 100644 include/qapi/qmp/qnull.h >>> >>> diff --git a/include/qapi/qmp/qnull.h b/include/qapi/qmp/qnull.h >>> new file mode 100644 >>> index 000..69555ac >>> --- /dev/null >>> +++ b/include/qapi/qmp/qnull.h >>> @@ -0,0 +1,26 @@ >>> +/* >>> + * QNull Module >>> + * >>> + * Copyright (C) 2009, 2017 Red Hat Inc. >>> + * >>> + * Authors: >>> + * Luiz Capitulino >>> + * >>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or >>> later. >>> + * See the COPYING.LIB file in the top-level directory. >> >> Copy the boilerplate from qnull.c instead, factual correctness. > > Sorry, will do. No need to be sorry! Copying it from qobject.h along with the code was the obvious thing to do. [...]
Re: [Qemu-block] [Qemu-devel] [PATCH v3] live-block-ops.txt: Rename, rewrite, and improve it
On 06/22/2017 04:56 AM, Kashyap Chamarthy wrote: > On Wed, Jun 21, 2017 at 06:49:02PM -0400, John Snow wrote: > > [...] > >>> * TODO (after feedback from John Snow): >>>- Eric Blake suggested to consider documenting incremental backup >>> policies as part of the section: "Live disk backup --- >>> `drive-backup` and `blockdev-backup`" >> >> Perhaps it could be mentioned, but hopefully I've covered it in some >> sufficient detail in the (now) docs/devel/bitmaps.md file; > > Yes, that doc is very useful. That was my precise thought. > >> I'm a little wary of duplicating efforts in this area, > > I share your wariness. > >> but you've covered everything *else* in good detail here, so now my >> file is the odd one out. >> >> I will leave this up to you, really. Perhaps it could be paid some lip >> service with a link to the other document? > > Yes, I was thinking of this, too -- just link to the 'bitmaps' document. > > A quick side question here: Since upstream QEMU is converging onto > Sphinx, and rST, hope you mind if I convert docs/devel/bitmaps.md into > rST at somepoint, for consistency's sake. I'll file a separate review, > anyway for that. In the long term, all / most other documents would > also be converted. > Of course not. I chose bitmaps.md so that it would be nice to view from the github interface while remaining nice to read in plaintext, but feel free to convert it if we actually do standardize on Sphinx/rST. If you can make the generated output look prettier than the github rendering of the markdown I'll ACK it ;) >> The detail in bitmaps.md is a little more verbose than the rest of >> this file, so if you include it wholesale it'd dwarf the rest of this >> document. >> >> What do you think? > > Yes, I fully agree with your suggestion. I will simply link to the > detailed document you wrote, which I was thinking of anyhow. > > Thanks for your comments! > Sure. You could perhaps mention the different sync modes, including top, none, full and incremental and urge readers to check out the bitmaps document for detailed workings of the incremental mode.
Re: [Qemu-block] [PATCH v2 2/4] qcow2: add qcow2_cache_discard
On 22.06.2017 01:29, Max Reitz wrote:
On 2017-06-13 14:16, Pavel Butsykin wrote:
Whenever l2/refcount table clusters are discarded from the file we can
automatically drop unnecessary content of the cache tables. This reduces
the chance of eviction useful cache data and eliminates inconsistent data
in thecache with the data in the file.
Signed-off-by: Pavel Butsykin
---
block/qcow2-cache.c| 21 +
block/qcow2-refcount.c | 5 +
block/qcow2.h | 1 +
3 files changed, 27 insertions(+)
diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 1d25147392..7931edf237 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -411,3 +411,24 @@ void qcow2_cache_entry_mark_dirty(BlockDriverState *bs,
Qcow2Cache *c,
assert(c->entries[i].offset != 0);
c->entries[i].dirty = true;
}
+
+void qcow2_cache_discard(BlockDriverState *bs, Qcow2Cache *c, uint64_t offset)
+{
+int i;
+
+for (i = 0; i < c->size; i++) {
+if (c->entries[i].offset == offset) {
+goto found; /* table offset */
+}
+}
+return;
+
+found:
+assert(c->entries[i].ref == 0);
+
+c->entries[i].offset = 0;
+c->entries[i].lru_counter = 0;
+c->entries[i].dirty = false;
+
+qcow2_cache_table_release(bs, c, i, 1);
+}
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 7c06061aae..576ab551d6 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -767,6 +767,11 @@ static int QEMU_WARN_UNUSED_RESULT
update_refcount(BlockDriverState *bs,
s->set_refcount(refcount_block, block_index, refcount);
if (refcount == 0 && s->discard_passthrough[type]) {
+qcow2_cache_put(bs, s->refcount_block_cache, &refcount_block);
I don't like this very much. It works, but it feels bad.
Would it be possible to store this refblock's offset somewhere and only
put it back if @offset is equal to that?
+qcow2_cache_discard(bs, s->refcount_block_cache, offset);
+
+qcow2_cache_discard(bs, s->l2_table_cache, offset);
+
So you're blindly calling qcow2_cache_discard() on @offset because
@offset may be pointing to a refblock or an L2 table? Right, that works,
but it still deserves a comment, I think (that we have no idea whether
@offset actually points to any of these refcount structures, but that we
also just don't have to care).
Looks OK to me, functionally, but I'd at least like to have that comment.
Hmm.. We can split qcow2_cache_discard() and kill two birds with one
stone.
void* qcow2_cache_is_table_offset(BlockDriverState *bs, Qcow2Cache *c,
uint64_t offset)
{
int i;
for (i = 0; i < c->size; i++) {
if (c->entries[i].offset == offset) {
return qcow2_cache_get_table_addr(bs, c, i);
}
}
return NULL;
}
void qcow2_cache_discard(BlockDriverState *bs, Qcow2Cache *c, void table)
{
int i = qcow2_cache_get_table_idx(bs, c, table);
assert(c->entries[i].ref == 0);
c->entries[i].offset = 0;
c->entries[i].lru_counter = 0;
c->entries[i].dirty = false;
qcow2_cache_table_release(bs, c, i, 1);
}
if (refcount == 0 && s->discard_passthrough[type]) {
void *table;
table = qcow2_cache_is_table_offset(bs, s->refcount_block_cache,
offset);
if (table != NULL) {
qcow2_cache_put(bs, s->refcount_block_cache, &refcount_block);
qcow2_cache_discard(bs, s->refcount_block_cache, table);
}
table = qcow2_cache_is_table_offset(bs, s->l2_table_cache, offset);
if (table != NULL) {
qcow2_cache_discard(bs, s->l2_table_cache, table);
}
...
Max
update_refcount_discard(bs, cluster_offset, s->cluster_size);
}
}
diff --git a/block/qcow2.h b/block/qcow2.h
index 1801dc30dc..07faa6dc78 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -597,5 +597,6 @@ int qcow2_cache_get(BlockDriverState *bs, Qcow2Cache *c,
uint64_t offset,
int qcow2_cache_get_empty(BlockDriverState *bs, Qcow2Cache *c, uint64_t
offset,
void **table);
void qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table);
+void qcow2_cache_discard(BlockDriverState *bs, Qcow2Cache *c, uint64_t offset);
#endif
Re: [Qemu-block] [PATCH v2 3/4] qcow2: add shrink image support
On 22.06.2017 01:55, Max Reitz wrote:
On 2017-06-13 14:16, Pavel Butsykin wrote:
This patch add shrinking of the image file for qcow2. As a result, this allows
us to reduce the virtual image size and free up space on the disk without
copying the image. Image can be fragmented and shrink is done by punching holes
in the image file.
Signed-off-by: Pavel Butsykin
---
block/qcow2-cluster.c | 42
block/qcow2-refcount.c | 65 ++
block/qcow2.c | 40 +++
block/qcow2.h | 2 ++
qapi/block-core.json | 3 ++-
5 files changed, 141 insertions(+), 11 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index d779ea19cf..a84b7e607e 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -32,6 +32,48 @@
#include "qemu/bswap.h"
#include "trace.h"
+int qcow2_shrink_l1_table(BlockDriverState *bs, uint64_t max_size)
It's not really a max_size but always an exact size. You don't want it
to be any smaller than this.
+{
+BDRVQcow2State *s = bs->opaque;
+int new_l1_size, i, ret;
+
+if (max_size >= s->l1_size) {
+return 0;
+}
+
+new_l1_size = max_size;
+
+#ifdef DEBUG_ALLOC2
+fprintf(stderr, "shrink l1_table from %d to %" PRId64 "\n",
+s->l1_size, new_l1_size);
new_l1_size is of type int, not int64_t.
+#endif
+
+BLKDBG_EVENT(bs->file, BLKDBG_L1_SHRINK_WRITE_TABLE);
+ret = bdrv_pwrite_zeroes(bs->file, s->l1_table_offset +
+ sizeof(uint64_t) * new_l1_size,
+ (s->l1_size - new_l1_size) * sizeof(uint64_t), 0);
+if (ret < 0) {
+return ret;
+}
+
+ret = bdrv_flush(bs->file->bs);
+if (ret < 0) {
+return ret;
+}
+
+BLKDBG_EVENT(bs->file, BLKDBG_L1_SHRINK_FREE_L2_CLUSTERS);
+for (i = s->l1_size - 1; i > new_l1_size - 1; i--) {
+if ((s->l1_table[i] & L1E_OFFSET_MASK) == 0) {
+continue;
+}
+qcow2_free_clusters(bs, s->l1_table[i] & L1E_OFFSET_MASK,
+s->l2_size * sizeof(uint64_t),
I'm more of a fan of s->cluster_size instead of s->l2_size *
sizeof(uint64_t) but it's not like it matters...
+QCOW2_DISCARD_ALWAYS);
+s->l1_table[i] = 0;
I'd probably clear the overhanging s->l1_table entries before
bdrv_flush() (before you shouldn't really use them after
bdrv_pwrite_zeroes() has returned, even if bdrv_flush() has failed), but
it's not absolutely necessary. As long as they still have a refcount of
at least one, writing to them will just be useless but not destroy any data.
You're right, but If it's not necessary, I would prefer to leave as is..
Just because overhanging s->l1_table entries used to release clusters :)
+}
+return 0;
+}
+
int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
bool exact_size)
{
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 576ab551d6..e98306acd8 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -29,6 +29,7 @@
#include "block/qcow2.h"
#include "qemu/range.h"
#include "qemu/bswap.h"
+#include "qemu/cutils.h"
static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size);
static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
@@ -2936,3 +2937,67 @@ done:
qemu_vfree(new_refblock);
return ret;
}
+
+int qcow2_shrink_reftable(BlockDriverState *bs)
+{
+BDRVQcow2State *s = bs->opaque;
+uint64_t *reftable_tmp =
+g_try_malloc(sizeof(uint64_t) * s->refcount_table_size);
+int i, ret;
+
+if (s->refcount_table_size && reftable_tmp == NULL) {
+return -ENOMEM;
+}
+
+for (i = 0; i < s->refcount_table_size; i++) {
+int64_t refblock_offs = s->refcount_table[i] & REFT_OFFSET_MASK;
+void *refblock;
+bool unused_block;
+
+if (refblock_offs == 0) {
+reftable_tmp[i] = 0;
+continue;
+}
+ret = qcow2_cache_get(bs, s->refcount_block_cache, refblock_offs,
+ &refblock);
+if (ret < 0) {
+goto out;
+}
+
+/* the refblock has own reference */
+if (i == refblock_offs >> (s->refcount_block_bits + s->cluster_bits)) {
+uint64_t blk_index = (refblock_offs >> s->cluster_bits) &
+ (s->refcount_block_size - 1);
+uint64_t refcount = s->get_refcount(refblock, blk_index);
+
+s->set_refcount(refblock, blk_index, 0);
+
+unused_block = buffer_is_zero(refblock, s->refcount_block_size);
s/refcount_block_size/cluster_size/
+
+s->set_refcount(refblock, blk_index, refcount);
+} else {
+unused_block = buffer_is_zero(refblock, s->refcount_block_size);
Same here.
+}
+q
Re: [Qemu-block] [Qemu-devel] [Question] How can we confirm hot-plug disk succesfully?
Kevin Wolf writes:
> Am 19.06.2017 um 12:27 hat Xie Changlong geschrieben:
>> 在 6/19/2017 3:27 PM, Kevin Wolf 写道:
>> >Am 18.06.2017 um 09:21 hat Xie Changlong geschrieben:
>> >>In device hot-remove scenario, if we don't probe acpiphp module on
>> >>the guest, 'device_del' will never emit DEVICE_DELETED event(because
>> >>guest will not write to __EJ0) . So we can confirm that hot-remove
>> >>failed. But IIUC, there is no event such as DEVICE_ADDED, so
>> >>1) How can we confirm hotplug('device_add') successfully?
>> >>2) It seems when hot-plug disk, we don't care acpiphp module status
>> >>on the guest, am I right?
>> >>3) Why there is no DEVICE_ADDED like event?
>> >
>> >device_add doesn't need guest cooperation, so it is immediately
>> >completed when the QMP command returns success.
>> >
>>
>> That's what i though too. But I have a question, if we don't proble
>> acpiphp on the guest, and execute below commands:
>>
>> Hot plug:
>> (qemu) drive_add 0
>> file=/resource/changlox/test.raw,id=drive-virtio-disk1,if=none
>> (qemu) device_add virtio-blk-pci,drive=drive-virtio-disk1,id=virtio-disk1
>>
>> Hot remove:
>> (qemu) device_del virtio-disk1
>> (qemu) drive_del drive-virtio-disk1
>> (qemu) qom-list /machine/peripheral
>> type (string)
>> virtio-disk1 (child)
>> (qemu) info block
>> foo (#block122): suse1.qcow2 (qcow2)
>> Cache mode: writeback, direct
>> Backing file: suse.qcow2.orgin (chain depth: 1)
>>
>> ide1-cd0: [not inserted]
>> Removable device: not locked, tray closed
>>
>> floppy0: [not inserted]
>> Removable device: not locked, tray closed
>>
>> sd0: [not inserted]
>> Removable device: not locked, tray closed
>> (qemu) drive_add 0
>> file=/resource/changlox/test.raw,id=drive-virtio-disk1,if=none
>> Duplicate ID 'drive-virtio-disk1' for drive
>>
>> 'info block' shows nothing, but we can't add drive who's id
>> is'drive-virtio-disk1' too.
>
> Yes, the old BlockBackend is only fully freed when the guest actually
> unplugs the device. Specifically, we would have to free the QemuOpts
> in DriveInfo that keeps the ID reserved. Currently, it is only freed
> when the BlockBackend is destroyed:
>
> blockdev_auto_del()
> blk_unref()
> blk_delete()
> drive_info_del()
>
> We can't free the DriveInfo earlier because it's still needed while the
> guest device is still alive.
>
> I'm not sure, but it may be possible to free just the QemuOpts in
> monitor_remove_blk(), so that the ID can immediately be reused.
>
> Markus, would you know?
It's complicated :)
= Abstract discussion =
QemuOpts enforces unique IDs.
The things configured by QemuOpts often also have unique IDs.
If a certain kind of thing is always created from a QemuOpts, and it
keeps its QemuOpts alive until it dies itself, then needn't enfore
unique IDs itself. If it does anyway, these errors are unreachable.
If it can be created without QemuOpts, or its QemuOpts can die early,
then it has to enforce unique IDs itself. This can lead to inconsistent
error messages: if the clash is detected by QemuOpts, QemuOpts reports
it, but if its only detected by the thing, the thing reports it.
If your thing enforces unique IDs itself, and nothing needs its
QemuOpts, then it can destroy it at any time. Note that -writeconfig
needs all QemuOpts created for command line options left of it.
With the benefit of hindsight: QemuOpts tries to do too much. It should
just parse option arguments (treating "id" like any other parameter) and
optionally accumulate parsed option arguments, so you can act on them
later (command line works that way).
= Concrete discussion =
Initially, every block backend had a unique name, but this has changed,
and I don't know for sure whether block backends rely in QemuOpts for
enforcing uniqueness now, and what else might need the QemuOpts.
HMP drive_del has awkward semantics that require removing the name from
the backend in some cases. I guess the QemuOpts ID should also go away
then, but I'm not sure, it's been too long since I messed around in that
particular swamp.
Sorry I can't give more definite answers.
>> There is a more serious situation, we
>> could *never* destory device memory with 'device_del', it's memory
>> leak to me if the guest os never support hot-plug and the user don't
>> know this information.
>
> The user sees that they never get a DEVICE_REMOVED event, so in some way
> the do know about it.
Concur. There is no leak, we simply can't unplug. We can't even tell
the user that we can't unplug. ACPI unplug is an afterthought, and it
shows.
Re: [Qemu-block] [PATCH v2 1/4] qemu-img: add --shrink flag for resize
On 22.06.2017 01:17, Max Reitz wrote:
On 2017-06-13 14:16, Pavel Butsykin wrote:
The flag as additional precaution of data loss. Perhaps in the future the
operation shrink without this flag will be banned, but while we need to
maintain compatibility.
Signed-off-by: Pavel Butsykin
---
qemu-img-cmds.hx | 4 ++--
qemu-img.c | 15 +++
qemu-img.texi | 5 -
tests/qemu-iotests/102 | 4 ++--
4 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index a39fcdba71..3b2eab9d20 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -76,9 +76,9 @@ STEXI
ETEXI
DEF("resize", img_resize,
-"resize [--object objectdef] [--image-opts] [-q] filename [+ | -]size")
+"resize [--object objectdef] [--image-opts] [-q] [--shrink] filename [+ |
-]size")
STEXI
-@item resize [--object @var{objectdef}] [--image-opts] [-q] @var{filename} [+
| -]@var{size}
+@item resize [--object @var{objectdef}] [--image-opts] [-q] [--shrink]
@var{filename} [+ | -]@var{size}
ETEXI
DEF("amend", img_amend,
diff --git a/qemu-img.c b/qemu-img.c
index 0ad698d7f1..bfe5f61b0b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -61,6 +61,7 @@ enum {
OPTION_FLUSH_INTERVAL = 261,
OPTION_NO_DRAIN = 262,
OPTION_TARGET_IMAGE_OPTS = 263,
+OPTION_SHRINK = 264,
};
typedef enum OutputFormat {
@@ -3452,6 +3453,7 @@ static int img_resize(int argc, char **argv)
},
};
bool image_opts = false;
+bool shrink = false;
/* Remove size from argv manually so that negative numbers are not treated
* as options by getopt. */
@@ -3469,6 +3471,7 @@ static int img_resize(int argc, char **argv)
{"help", no_argument, 0, 'h'},
{"object", required_argument, 0, OPTION_OBJECT},
{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+{"shrink", no_argument, 0, OPTION_SHRINK},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, ":f:hq",
@@ -3503,6 +3506,9 @@ static int img_resize(int argc, char **argv)
case OPTION_IMAGE_OPTS:
image_opts = true;
break;
+case OPTION_SHRINK:
+shrink = true;
+break;
}
}
if (optind != argc - 1) {
@@ -3562,6 +3568,15 @@ static int img_resize(int argc, char **argv)
goto out;
}
+if (total_size < blk_getlength(blk) && !shrink) {
+qprintf(quiet, "Warning: shrinking of the image can lead to data loss.
"
I think this should always be printed to stderr, even if quiet is true;
especially considering we (or at least I) plan to make it mandatory.
OK.
+ "Before performing shrink operation you must make sure "
*Before performaing a shrink operation
s/performaing/performing/
Also, I'd rather use the imperative: "Before ... operation, make sure
that the..."
(English isn't my native language either, but as far as I remember
"must" is generally used for external influences. But it's not like a
force of nature is forcing you to confirm there's no important data; you
can just ignore this advice and see all of your non-backed-up childhood
pictures go to /dev/null.)
Yes, It should be just a recommendation.
(As you might have already guessed, English isn't my native language too
:) I'm glad you are understanding. Really thanks for helping to improve
the text.)
+ "that the shrink part of image doesn't contain
important"
Hmm... I don't think "shrink part" really works.
Maybe the following would work better:
Warning: Shrinking an image will delete all data beyond the shrunken
image's end. Before performing such an operation, make sure there is
no important data there.
+ " data.\n");
+qprintf(quiet,
+"If you don't want to see this message use --shrink
option.\n");
You should make a note that --shrink may (and I hope it will) become
necessary in the future, like:
Using the --shrink option will suppress this message. Note that future
versions of qemu-img may refuse to shrink images without this option!
will fix.
+}
+
ret = blk_truncate(blk, total_size, &err);
if (!ret) {
qprintf(quiet, "Image resized.\n");
diff --git a/qemu-img.texi b/qemu-img.texi
index 5b925ecf41..c2b694cd00 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -499,7 +499,7 @@ qemu-img rebase -b base.img diff.qcow2
At this point, @code{modified.img} can be discarded, since
@code{base.img + diff.qcow2} contains the same information.
-@item resize @var{filename} [+ | -]@var{size}
+@item resize [--shrink] @var{filename} [+ | -]@var{size}
Change the disk image as if it had been created with @var{size}.
@@ -507,6 +507,9 @@ Before using this command to shrink a disk image, you MUST use file system and
partitioning tools inside the VM to reduce allocated file syst
[Qemu-block] [PATCH 08/31] qcow2: use DIV_ROUND_UP
I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra
Signed-off-by: Marc-André Lureau
---
block/qcow2-cluster.c | 2 +-
block/qcow2-refcount.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index d779ea19cf..da9008815c 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -61,7 +61,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t
min_size,
new_l1_size = 1;
}
while (min_size > new_l1_size) {
-new_l1_size = (new_l1_size * 3 + 1) / 2;
+new_l1_size = DIV_ROUND_UP(new_l1_size * 3, 2);
}
}
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 7c06061aae..75107cf093 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -293,7 +293,7 @@ static unsigned int next_refcount_table_size(BDRVQcow2State
*s,
MAX(1, s->refcount_table_size >> (s->cluster_bits - 3));
while (min_clusters > refcount_table_clusters) {
-refcount_table_clusters = (refcount_table_clusters * 3 + 1) / 2;
+refcount_table_clusters = DIV_ROUND_UP(refcount_table_clusters * 3, 2);
}
return refcount_table_clusters << (s->cluster_bits - 3);
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 10/31] vvfat: use DIV_ROUND_UP
I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra
Signed-off-by: Marc-André Lureau
---
block/vvfat.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index 426ca70e35..877f71dcdc 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -433,7 +433,7 @@ static inline direntry_t*
create_long_filename(BDRVVVFATState* s,const char* fil
{
char buffer[258];
int length=short2long_name(buffer,filename),
-number_of_entries=(length+25)/26,i;
+number_of_entries=DIV_ROUND_UP(length, 26),i;
direntry_t* entry;
for(i=0;i offset && c >=2 && !fat_eof(s, c)));
ret = vvfat_read(s->bs, cluster2sector(s, c),
- (uint8_t*)cluster, (rest_size + 0x1ff) / 0x200);
+ (uint8_t*)cluster, DIV_ROUND_UP(rest_size, 0x200));
if (ret < 0) {
qemu_close(fd);
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 09/31] vpc: use DIV_ROUND_UP
I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra
Signed-off-by: Marc-André Lureau
---
block/vpc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/vpc.c b/block/vpc.c
index 4240ba9d1c..f52a7c0f0f 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -760,7 +760,7 @@ static int calculate_geometry(int64_t total_sectors,
uint16_t* cyls,
} else {
*secs_per_cyl = 17;
cyls_times_heads = total_sectors / *secs_per_cyl;
-*heads = (cyls_times_heads + 1023) / 1024;
+*heads = DIV_ROUND_UP(cyls_times_heads, 1024);
if (*heads < 4) {
*heads = 4;
@@ -813,7 +813,7 @@ static int create_dynamic_disk(BlockBackend *blk, uint8_t
*buf,
offset = 3 * 512;
memset(buf, 0xFF, 512);
-for (i = 0; i < (num_bat_entries * 4 + 511) / 512; i++) {
+for (i = 0; i < DIV_ROUND_UP(num_bat_entries * 4, 512); i++) {
ret = blk_pwrite(blk, offset, buf, 512, 0);
if (ret < 0) {
goto fail;
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 07/31] dmg: use DIV_ROUND_UP
I used the clang-tidy qemu-round check to generate the fix: https://github.com/elmarco/clang-tools-extra Signed-off-by: Marc-André Lureau --- block/dmg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/dmg.c b/block/dmg.c index 900ae5a678..6c0711f563 100644 --- a/block/dmg.c +++ b/block/dmg.c @@ -111,7 +111,7 @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk, uncompressed_sectors = s->sectorcounts[chunk]; break; case 1: /* copy */ -uncompressed_sectors = (s->lengths[chunk] + 511) / 512; +uncompressed_sectors = DIV_ROUND_UP(s->lengths[chunk], 512); break; case 2: /* zero */ /* as the all-zeroes block may be large, it is treated specially: the -- 2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 03/31] vhdx: use QEMU_ALIGN_DOWN
I used the clang-tidy qemu-round check to generate the fix: https://github.com/elmarco/clang-tools-extra Signed-off-by: Marc-André Lureau --- block/vhdx-log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/vhdx-log.c b/block/vhdx-log.c index 3f4c2aa095..6125ea4c6d 100644 --- a/block/vhdx-log.c +++ b/block/vhdx-log.c @@ -884,7 +884,7 @@ static int vhdx_log_write(BlockDriverState *bs, BDRVVHDXState *s, } sector_offset = offset % VHDX_LOG_SECTOR_SIZE; -file_offset = (offset / VHDX_LOG_SECTOR_SIZE) * VHDX_LOG_SECTOR_SIZE; +file_offset = QEMU_ALIGN_DOWN(offset, VHDX_LOG_SECTOR_SIZE); aligned_length = length; -- 2.13.1.395.gf7b71de06
[Qemu-block] [PATCH] docs: add qemu-block-drivers(7) man page
Block driver documentation is available in qemu-doc.html. It would be convenient to have documentation for formats, protocols, and filter drivers in a man page. Extract the relevant part of qemu-doc.html into a new file called docs/qemu-block-drivers.texi. This file can also be built as a stand-alone document (man, html, etc). Signed-off-by: Stefan Hajnoczi --- Makefile | 28 +- docs/qemu-block-drivers.texi | 692 +++ qemu-doc.texi| 673 + 3 files changed, 712 insertions(+), 681 deletions(-) create mode 100644 docs/qemu-block-drivers.texi diff --git a/Makefile b/Makefile index c830d7a..ee9b434 100644 --- a/Makefile +++ b/Makefile @@ -209,6 +209,9 @@ ifdef BUILD_DOCS DOCS=qemu-doc.html qemu-doc.txt qemu.1 qemu-img.1 qemu-nbd.8 qemu-ga.8 DOCS+=docs/qemu-qmp-ref.html docs/qemu-qmp-ref.txt docs/qemu-qmp-ref.7 DOCS+=docs/qemu-ga-ref.html docs/qemu-ga-ref.txt docs/qemu-ga-ref.7 +DOCS+=docs/qemu-block-drivers.html +DOCS+=docs/qemu-block-drivers.txt +DOCS+=docs/qemu-block-drivers.7 ifdef CONFIG_VIRTFS DOCS+=fsdev/virtfs-proxy-helper.1 endif @@ -520,10 +523,10 @@ distclean: clean rm -f config.log rm -f linux-headers/asm rm -f docs/qemu-ga-qapi.texi docs/qemu-qmp-qapi.texi docs/version.texi - rm -f docs/qemu-qmp-ref.7 docs/qemu-ga-ref.7 - rm -f docs/qemu-qmp-ref.txt docs/qemu-ga-ref.txt - rm -f docs/qemu-qmp-ref.pdf docs/qemu-ga-ref.pdf - rm -f docs/qemu-qmp-ref.html docs/qemu-ga-ref.html + rm -f docs/qemu-qmp-ref.7 docs/qemu-ga-ref.7 docs/qemu-block-drivers.7 + rm -f docs/qemu-qmp-ref.txt docs/qemu-ga-ref.txt docs/qemu-block-drivers.txt + rm -f docs/qemu-qmp-ref.pdf docs/qemu-ga-ref.pdf docs/qemu-block-drivers.pdf + rm -f docs/qemu-qmp-ref.html docs/qemu-ga-ref.html docs/qemu-block-drivers.html for d in $(TARGET_DIRS); do \ rm -rf $$d || exit 1 ; \ done @@ -564,11 +567,14 @@ install-doc: $(DOCS) $(INSTALL_DATA) qemu-doc.txt "$(DESTDIR)$(qemu_docdir)" $(INSTALL_DATA) docs/qemu-qmp-ref.html "$(DESTDIR)$(qemu_docdir)" $(INSTALL_DATA) docs/qemu-qmp-ref.txt "$(DESTDIR)$(qemu_docdir)" + $(INSTALL_DATA) docs/qemu-block-drivers.html "$(DESTDIR)$(qemu_docdir)" + $(INSTALL_DATA) docs/qemu-block-drivers.txt "$(DESTDIR)$(qemu_docdir)" ifdef CONFIG_POSIX $(INSTALL_DIR) "$(DESTDIR)$(mandir)/man1" $(INSTALL_DATA) qemu.1 "$(DESTDIR)$(mandir)/man1" $(INSTALL_DIR) "$(DESTDIR)$(mandir)/man7" $(INSTALL_DATA) docs/qemu-qmp-ref.7 "$(DESTDIR)$(mandir)/man7" + $(INSTALL_DATA) docs/qemu-block-drivers.7 "$(DESTDIR)$(mandir)/man7" ifneq ($(TOOLS),) $(INSTALL_DATA) qemu-img.1 "$(DESTDIR)$(mandir)/man1" $(INSTALL_DIR) "$(DESTDIR)$(mandir)/man8" @@ -688,6 +694,7 @@ docs/version.texi: $(SRC_PATH)/VERSION docs/qemu-ga-ref.html docs/qemu-ga-ref.info docs/qemu-ga-ref.txt docs/qemu-ga-ref.pdf docs/qemu-ga-ref.7.pod: docs/version.texi docs/qemu-qmp-ref.html docs/qemu-qmp-ref.info docs/qemu-qmp-ref.txt docs/qemu-qmp-ref.pdf docs/qemu-qmp-ref.pod: docs/version.texi +docs/qemu-block-drivers.html docs/qemu-block-drivers.info docs/qemu-block-drivers.txt docs/qemu-block-drivers.pdf docs/qemu-block-drivers.pod: docs/version.texi qemu-options.texi: $(SRC_PATH)/qemu-options.hx $(SRC_PATH)/scripts/hxtool $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t < $< > $@,"GEN","$@") @@ -716,15 +723,15 @@ fsdev/virtfs-proxy-helper.1: fsdev/virtfs-proxy-helper.texi qemu-nbd.8: qemu-nbd.texi qemu-option-trace.texi qemu-ga.8: qemu-ga.texi -html: qemu-doc.html docs/qemu-qmp-ref.html docs/qemu-ga-ref.html -info: qemu-doc.info docs/qemu-qmp-ref.info docs/qemu-ga-ref.info -pdf: qemu-doc.pdf docs/qemu-qmp-ref.pdf docs/qemu-ga-ref.pdf -txt: qemu-doc.txt docs/qemu-qmp-ref.txt docs/qemu-ga-ref.txt +html: qemu-doc.html docs/qemu-qmp-ref.html docs/qemu-ga-ref.html docs/qemu-block-drivers.html +info: qemu-doc.info docs/qemu-qmp-ref.info docs/qemu-ga-ref.info docs/qemu-block-drivers.info +pdf: qemu-doc.pdf docs/qemu-qmp-ref.pdf docs/qemu-ga-ref.pdf docs/qemu-block-drivers.pdf +txt: qemu-doc.txt docs/qemu-qmp-ref.txt docs/qemu-ga-ref.txt docs/qemu-block-drivers.txt qemu-doc.html qemu-doc.info qemu-doc.pdf qemu-doc.txt: \ qemu-img.texi qemu-nbd.texi qemu-options.texi qemu-option-trace.texi \ qemu-monitor.texi qemu-img-cmds.texi qemu-ga.texi \ - qemu-monitor-info.texi + qemu-monitor-info.texi docs/qemu-block-drivers.texi docs/qemu-ga-ref.dvi docs/qemu-ga-ref.html docs/qemu-ga-ref.info docs/qemu-ga-ref.pdf docs/qemu-ga-ref.txt docs/qemu-ga-ref.7: \ docs/qemu-ga-ref.texi docs/qemu-ga-qapi.texi @@ -732,6 +739,9 @@ docs/qemu-ga-ref.texi docs/qemu-ga-qapi.texi docs/qemu-qmp-ref.dvi docs/qemu-qmp-ref.html docs/qemu-qmp-ref.info docs/qemu-qmp-ref.pdf docs/qemu-qmp-ref.txt docs/qemu-qmp-ref.7: \ docs/qemu-q
Re: [Qemu-block] [Qemu-devel] [PATCH v3] live-block-ops.txt: Rename, rewrite, and improve it
On Wed, Jun 21, 2017 at 06:49:02PM -0400, John Snow wrote: [...] > > * TODO (after feedback from John Snow): > >- Eric Blake suggested to consider documenting incremental backup > > policies as part of the section: "Live disk backup --- > > `drive-backup` and `blockdev-backup`" > > Perhaps it could be mentioned, but hopefully I've covered it in some > sufficient detail in the (now) docs/devel/bitmaps.md file; Yes, that doc is very useful. That was my precise thought. > I'm a little wary of duplicating efforts in this area, I share your wariness. > but you've covered everything *else* in good detail here, so now my > file is the odd one out. > > I will leave this up to you, really. Perhaps it could be paid some lip > service with a link to the other document? Yes, I was thinking of this, too -- just link to the 'bitmaps' document. A quick side question here: Since upstream QEMU is converging onto Sphinx, and rST, hope you mind if I convert docs/devel/bitmaps.md into rST at somepoint, for consistency's sake. I'll file a separate review, anyway for that. In the long term, all / most other documents would also be converted. > The detail in bitmaps.md is a little more verbose than the rest of > this file, so if you include it wholesale it'd dwarf the rest of this > document. > > What do you think? Yes, I fully agree with your suggestion. I will simply link to the detailed document you wrote, which I was thinking of anyhow. Thanks for your comments! -- /kashyap
