qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Peter Lieven <pl@kamp.de>
Cc: Kevin Wolf <kwolf@redhat.com>,
	ronniesahlberg@gmail.com, qemu-devel@nongnu.org,
	stefanha@redhat.com
Subject: Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert
Date: Wed, 17 Jul 2013 12:45:58 +0200	[thread overview]
Message-ID: <51E675E6.7050101@redhat.com> (raw)
In-Reply-To: <74B76DD7-FBF6-42CD-8B9D-62661B98A860@kamp.de>

Il 17/07/2013 12:21, Peter Lieven ha scritto:
>> > BTW, Peter and Ronnie: we were assuming that UNMAP with LBPRZ=1 always
>> > zeroes blocks, but is that true for unaligned operations?
> Good question, I will pass it to ronnie. My guess is that the command will fail with
> a check condition if it failed to unmap the data. From what Ronnie sent earlier
> it should be guaranteed that the blocks are at least zero after the unmap command.

I'm not so sure about that, SBC says explicitly that a misaligned unmap
request "may result in unmap operations on fewer LBAs than requested".

Perhaps it's better to do this:

- if LBPWS=1, use WRITE SAME(16).  For WRITE SAME it is explicit that
"if the device server does not deallocate or anchor the LBA, then the
device server shall perform the specified write operation".  Expose the
value of LBPRZ in the "discard zeroes" field of BlockDriverInfo.

- if LBPWS=0, you can still support discard iff LBPU=1 and use UNMAP.
But if LBPRZ=1 and LBPWS=0, you should not expose the "discard zeroes"
field of BlockDriverInfo.

> As for the qemu-img patch this shouldn't matter. It uses always blocks of bdi->max_unmap
> which should be a multiple of the alignment.

Also, the remainder itself (which SBC calls "UNMAP GRANULARITY
ALIGNMENT") may not be 0.  I think the above trick (only exposing LBPRZ
if you'll use WRITE SAME) will sidestep the problem.  It may also remove
the need to fall back to has_zero_init=1.

Paolo

  parent reply	other threads:[~2013-07-17 10:46 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-15 10:49 [Qemu-devel] [PATCH 0/4] qemu-img: conditionally discard target on convert Peter Lieven
2013-07-15 10:49 ` [Qemu-devel] [PATCH 1/4] block: add discard_zeroes and max_unmap to BlockDriverInfo Peter Lieven
2013-07-15 10:49 ` [Qemu-devel] [PATCH 2/4] iscsi: add .bdrv_get_info Peter Lieven
2013-07-15 10:49 ` [Qemu-devel] [PATCH 3/4] block/raw: " Peter Lieven
2013-07-19  5:12   ` Stefan Hajnoczi
2013-07-15 10:49 ` [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert Peter Lieven
2013-07-17  8:46   ` Kevin Wolf
2013-07-17  9:58     ` Paolo Bonzini
2013-07-17 10:21       ` Peter Lieven
2013-07-17 10:27         ` Kevin Wolf
2013-07-17 10:31           ` Peter Lieven
2013-07-17 10:47           ` Paolo Bonzini
2013-07-17 14:19             ` Peter Lieven
2013-07-17 10:45         ` Paolo Bonzini [this message]
2013-07-17 14:21           ` Peter Lieven
2013-07-17 14:26             ` Paolo Bonzini
2013-07-17 10:25       ` Kevin Wolf
2013-07-17 15:54         ` ronnie sahlberg
2013-07-17 16:27           ` Paolo Bonzini
2013-07-17 16:31             ` ronnie sahlberg
2013-07-17 17:02               ` Peter Lieven
2013-07-17 17:04                 ` Paolo Bonzini
2013-07-17 17:48                   ` Peter Lieven
2013-07-17 20:00                     ` Paolo Bonzini
2013-07-18  9:23                     ` Kevin Wolf
2013-07-18 10:24                       ` Paolo Bonzini
2013-07-18 10:42                         ` Kevin Wolf
2013-07-18 10:44                         ` Peter Lieven
2013-07-18 10:56                           ` Paolo Bonzini
2013-07-18 11:04                             ` Peter Lieven
2013-07-18 12:31                               ` Paolo Bonzini
2013-07-18 13:29                                 ` Peter Lieven
2013-07-18 13:52                                   ` Paolo Bonzini
2013-07-18 14:09                                     ` Peter Lieven
2013-07-18 14:20                                       ` Paolo Bonzini
2013-07-18 14:32                                         ` Peter Lieven
2013-07-18 14:35                                           ` Paolo Bonzini
2013-07-18 18:43                                             ` Peter Lieven
2013-07-18 18:54                                               ` ronnie sahlberg
2013-07-18 19:28                                                 ` Peter Lieven
2013-07-19  5:58                                                   ` Paolo Bonzini
2013-07-19  6:08                                                     ` Peter Lieven
2013-07-19 13:25                                                       ` ronnie sahlberg
2013-07-19 13:49                                                         ` Peter Lieven
2013-07-19 14:00                                                           ` ronnie sahlberg
2013-07-19 14:02                                                             ` Peter Lieven
2013-07-19 13:58                                               ` ronnie sahlberg
2013-07-18 13:55                                   ` ronnie sahlberg
2013-07-18 14:14                                     ` Paolo Bonzini
2013-09-12  8:52                                       ` Peter Lieven
2013-09-12  8:55                                         ` Paolo Bonzini
2013-09-12  9:03                                           ` Peter Lieven
2013-07-17 17:09                 ` ronnie sahlberg
2013-07-18  9:21           ` Kevin Wolf
2013-07-17 10:22     ` Peter Lieven
2013-07-16 10:55 ` [Qemu-devel] [PATCH 0/4] " Paolo Bonzini
2013-07-16 11:18   ` Peter Lieven
2013-07-16 11:27     ` Paolo Bonzini
2013-07-16 11:40       ` Peter Lieven
2013-07-16 11:55         ` Paolo Bonzini
2013-07-17 10:23           ` Peter Lieven
2013-07-17 10:28             ` Paolo Bonzini
2013-07-17 10:40               ` Peter Lieven
2013-07-17 10:50                 ` Paolo Bonzini
2013-07-17 14:18                   ` Peter Lieven
2013-07-17 14:26                     ` Paolo Bonzini
2013-07-17 14:46                       ` Peter Lieven
2013-07-17 14:53                         ` Paolo Bonzini
2013-07-17 15:12                           ` Kevin Wolf
2013-07-17 16:53                             ` Peter Lieven
2013-07-17 17:01                           ` Peter Lieven
2013-07-19  5:13 ` Stefan Hajnoczi

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=51E675E6.7050101@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pl@kamp.de \
    --cc=qemu-devel@nongnu.org \
    --cc=ronniesahlberg@gmail.com \
    --cc=stefanha@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).