All of lore.kernel.org
 help / color / mirror / Atom feed
From: Fam Zheng <famz@redhat.com>
To: "Denis V. Lunev" <den@virtuozzo.com>
Cc: kwolf@redhat.com,
	Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	Stefan Hajnoczi <stefanha@gmail.com>,
	qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	jsnow@redhat.com
Subject: Re: [Qemu-devel] [PATCH v2 0/6] external backup api
Date: Sat, 27 Feb 2016 12:26:13 +0800	[thread overview]
Message-ID: <20160227042613.GA12620@ad.usersys.redhat.com> (raw)
In-Reply-To: <56D0B858.6010303@virtuozzo.com>

On Fri, 02/26 23:40, Denis V. Lunev wrote:
> On 02/26/2016 10:55 PM, Paolo Bonzini wrote:
> >
> >On 19/02/2016 09:51, Markus Armbruster wrote:
> >>>Is it an abuse to "Get LBA Status" to return dirty information? Because in SCSI
> >>>the command reports "mapped", "allocated" and "anchored" statuses. Does that
> >>>mean NBD will use a different status set?
> >>Perhaps some conceptual gymnastics can get us to standard semantics.
> >>
> >>Incremental backup wants to copy out an image's "dirty" blocks.
> >>
> >>We can view this as a bitmap telling us which of the image's blocks are
> >>dirty.
> >>
> >>An alternative view would be base image + dirty delta image.  In the the
> >>dirty delta, exactly the dirty blocks are allocated.  The delta image
> >>may be conceptual.
> >I see a problem here. On one hand I agree that the "GET LBA STATUS" is
> >a natural extension to the NBD protocol.
> >
> >On the other hand, the Get LBA Status command in SCSI reflects the
> >state over the whole chain, not only the top element.  It is the
> >equivalent of bdrv_get_block_status_above(bs, NULL, ...), rather than
> >bdrv_get_block_status(bs, ...).  My understanding is that the dirty
> >block information would require the latter, especially in the
> >"conceptual delta image" model that Markus describes above.
> >
> >Having NBD implement subtly different semantics compared to SCSI is a
> >bad idea in my opinion.
> >
> >Of course if we call it "READ DIRTY BLOCKS" that would work, but I
> >don't think such a command would be something that it makes sense to
> >have in the general purpose NBD spec, so you would need a mechanism
> >for vendor-specific extensions.
> >
> >Paolo
> >
> In general, the idea to bind DIRTY BITMAP to GET STATUS command is not that
> bad. First of all, NBD has no relation to the SCSI at all thus we are not
> bound to the SCSI protocol. This is good thing.
> 
> Next, it is generally good to query state of the data block before reading
> to reduce amount of transfers over the network. This is useful even for a
> full backup operation to avoid zero block transfers.
> 
> Thus, if we have created special NBD share, we can provide STATUS bitmap
> in the way we want, f.e. with the proposed semantics. This will not be a
> violation of the protocol.

Paolo described what I was trying to say, but way clearer then what I said. :)
And I agree a dedicated bitmap status seems better here.

Fam

  reply	other threads:[~2016-02-27  4:26 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-30 10:56 [Qemu-devel] [PATCH v2 0/6] external backup api Vladimir Sementsov-Ogievskiy
2016-01-30 10:56 ` [Qemu-devel] [PATCH 1/6] block dirty bitmap: add next_zero function Vladimir Sementsov-Ogievskiy
2016-01-30 10:56 ` [Qemu-devel] [PATCH 2/6] qmp: add query-block-dirty-bitmap-ranges Vladimir Sementsov-Ogievskiy
2016-02-10 10:08   ` Stefan Hajnoczi
2016-02-10 13:57     ` Denis V. Lunev
2016-02-10 15:26       ` John Snow
2016-02-10 15:36         ` Denis V. Lunev
2016-02-10 15:37           ` John Snow
2016-02-10 15:40             ` Denis V. Lunev
2016-02-14  5:05       ` Fam Zheng
2016-01-30 10:56 ` [Qemu-devel] [PATCH 3/6] iotests: test query-block-dirty-bitmap-ranges Vladimir Sementsov-Ogievskiy
2016-01-30 10:56 ` [Qemu-devel] [PATCH 4/6] qapi: add qmp commands for some dirty bitmap functions Vladimir Sementsov-Ogievskiy
2016-01-30 10:56 ` [Qemu-devel] [PATCH 5/6] qapi: make block-dirty-bitmap-create-successor transaction-able Vladimir Sementsov-Ogievskiy
2016-01-30 10:56 ` [Qemu-devel] [PATCH 6/6] iotests: test external backup api Vladimir Sementsov-Ogievskiy
2016-02-03  8:14 ` [Qemu-devel] [PATCH v2 0/6] " Fam Zheng
2016-02-03 10:57   ` Vladimir Sementsov-Ogievskiy
2016-02-03 11:02     ` Fam Zheng
2016-02-03 11:24       ` Vladimir Sementsov-Ogievskiy
2016-02-05  8:28   ` Denis V. Lunev
2016-02-05  8:44     ` Fam Zheng
2016-02-09 14:21     ` Stefan Hajnoczi
2016-02-09 14:37       ` Denis V. Lunev
2016-02-09 16:49         ` John Snow
2016-02-09 16:58           ` Denis V. Lunev
2016-02-09 18:12             ` John Snow
2016-02-09 19:25               ` Denis V. Lunev
2016-02-10  8:04                 ` Denis V. Lunev
2016-02-09 14:28     ` Stefan Hajnoczi
2016-02-09 14:41       ` Denis V. Lunev
2016-02-10 10:10         ` Stefan Hajnoczi
2016-02-16 17:09           ` Stefan Hajnoczi
2016-02-16 17:17             ` Vladimir Sementsov-Ogievskiy
2016-02-16 17:20             ` Denis V. Lunev
2016-02-18 16:39               ` Stefan Hajnoczi
2016-02-18 17:07                 ` Markus Armbruster
2016-02-17 17:47             ` Vladimir Sementsov-Ogievskiy
2016-02-18  0:59               ` Fam Zheng
2016-02-18 12:11               ` Daniel P. Berrange
2016-02-18 16:41                 ` Stefan Hajnoczi
2016-02-19  2:08                   ` Fam Zheng
2016-02-19  8:51                     ` Markus Armbruster
2016-02-24 23:34                       ` John Snow
2016-02-26 19:55                       ` Paolo Bonzini
2016-02-26 20:03                         ` Paolo Bonzini
2016-02-26 20:29                           ` Denis V. Lunev
2016-02-26 21:37                           ` John Snow
2016-02-26 20:40                         ` Denis V. Lunev
2016-02-27  4:26                           ` Fam Zheng [this message]
2016-02-29  8:14                         ` Markus Armbruster
2016-02-29  8:54                           ` Paolo Bonzini
2016-02-29  9:42                             ` Paolo Bonzini
2016-02-29 10:05                               ` Fam Zheng
2016-03-10 17:37                               ` Stefan Hajnoczi
2016-03-10 17:40                                 ` Paolo Bonzini
2016-03-14 16:27                                   ` Denis V. Lunev
2016-02-29 10:22                           ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160227042613.GA12620@ad.usersys.redhat.com \
    --to=famz@redhat.com \
    --cc=armbru@redhat.com \
    --cc=den@virtuozzo.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.