qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Max Reitz <mreitz@redhat.com>
Cc: Eric Blake <eblake@redhat.com>, Fam Zheng <famz@redhat.com>,
	Kevin Wolf <kwolf@redhat.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v1 3/6] qemu-img: add support for -n arg to dd command
Date: Wed, 1 Feb 2017 12:40:14 +0000	[thread overview]
Message-ID: <20170201124014.GH3232@redhat.com> (raw)
In-Reply-To: <37f445c0-c174-bca4-7073-7aa6c64046b0@redhat.com>

On Wed, Feb 01, 2017 at 01:31:01PM +0100, Max Reitz wrote:
> On 01.02.2017 13:28, Daniel P. Berrange wrote:
> > On Wed, Feb 01, 2017 at 01:23:54PM +0100, Max Reitz wrote:
> >> On 01.02.2017 13:16, Daniel P. Berrange wrote:
> >>> On Wed, Feb 01, 2017 at 01:13:39PM +0100, Max Reitz wrote:
> >>>> On 30.01.2017 19:37, Eric Blake wrote:
> >>>>> On 01/26/2017 07:27 AM, Daniel P. Berrange wrote:
> >>>>>> On Thu, Jan 26, 2017 at 08:35:30PM +0800, Fam Zheng wrote:
> >>>>>>> On Thu, 01/26 11:04, Daniel P. Berrange wrote:
> >>>>>>>> The -n arg to the convert command allows use of a pre-existing image,
> >>>>>>>> rather than creating a new image. This adds a -n arg to the dd command
> >>>>>>>> to get feature parity.
> >>>>>>>
> >>>>>>> I remember there was a discussion about changing qemu-img dd's default to a
> >>>>>>> "conv=nocreat" semantic, if so, "-n" might not be that useful. But that part
> >>>>>>> hasn't made it into the tree, and I'm not sure which direction we should take.
> >>>>>>> (Personally I think default to nocreat is a good idea).
> >>>>>>
> >>>>>> Use nocreat by default would be semantically different from real "dd"
> >>>>>> binary which feels undesirable if the goal is to make "qemu-img dd"
> >>>>>> be as consistent with "dd" as possible.
> >>>>>>
> >>>>>> It would be trivial to rewrite this patch to add support for the "conv"
> >>>>>> option, allowing the user to explicitly give 'qemu-img dd conv=nocreat'
> >>>>>> instead of my 'qemu-img dd -n' syntax, without changing default semantics.
> >>>>>
> >>>>> Adding 'conv=nocreat' (and not '-n') feels like the right way to me.
> >>>>
> >>>> The original idea was to make conv=nocreat a mandatory option, I think.
> >>>> qemu-img was supposed error out if the user did not specify it.
> >>>
> >>> I'm not really seeing a benefit in doing that - it would just break
> >>> existing usage of qemu-img dd for no obvious benefit.
> >>
> >> Well... Is there existing usage?
> > 
> > It shipped in 2.8.0 though, so imho that means we have to assume there
> > are users, and thus additions must to be backwards compatible from now
> > on.
> 
> Depends. I don't think there are too many users, so we could still
> justify a change if there's a very good reason for it.
> 
> I do agree that it's probably not a very good reason, though.
> 
> >> The benefit would be that one could (should?) expect qemu-img dd to
> >> behave on disk images as if they were block devices; and dd to a block
> >> device will not truncate or "recreate" it.
> >>
> >> If you don't give nocreat, it's thus a bit unclear whether you want to
> >> delete and recreate the target or whether you want to write into it.
> >> Some may expect qemu-img dd to behave as if the target is a normal file
> >> (delete and recreate it), others may expect it's treated like a block
> >> device (just write into it). If you force the user to specify nocreat,
> >> it would make the behavior clear.
> >>
> >> (And you can always delete+recreate the target with qemu-img create.)
> >>
> >> It's all a bit complicated. :-/
> > 
> > If the goal is to be compatible with /usr/bin/dd then IIUC, the behaviour
> > needs to be
> > 
> >  - If target is a block device, then silently assume nocreat|notrunc
> >    is set, even if not specified by user
> > 
> >  - If target is a file, then silently create & truncate the file
> >    unless nocreat or notrunc are set
> 
> Yes. But you could easily argue that every image file is a "block device".

IMHO that would be a bad idea as it would mean different behaviour
from dd vs qemu-img dd, when run on raw files.

If we assume nocreat|notrunc behaviour by default, then we would  likely
need to invent new "creat|trunc" flags to let people turn the previous
behaviour back on, which would diverge from 'dd' command.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

  reply	other threads:[~2017-02-01 12:40 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-26 11:04 [Qemu-devel] [PATCH v1 0/6] qemu-img: improve convert & dd commands Daniel P. Berrange
2017-01-26 11:04 ` [Qemu-devel] [PATCH v1 1/6] qemu-img: add support for --object with 'dd' command Daniel P. Berrange
2017-01-30 16:48   ` Eric Blake
2017-01-26 11:04 ` [Qemu-devel] [PATCH v1 2/6] qemu-img: fix --image-opts usage with dd command Daniel P. Berrange
2017-01-26 12:28   ` Fam Zheng
2017-01-26 11:04 ` [Qemu-devel] [PATCH v1 3/6] qemu-img: add support for -n arg to " Daniel P. Berrange
2017-01-26 12:35   ` Fam Zheng
2017-01-26 13:27     ` Daniel P. Berrange
2017-01-28 11:55       ` Fam Zheng
2017-01-30 18:37       ` Eric Blake
2017-02-01 12:13         ` Max Reitz
2017-02-01 12:16           ` Daniel P. Berrange
2017-02-01 12:23             ` Max Reitz
2017-02-01 12:28               ` Daniel P. Berrange
2017-02-01 12:31                 ` Max Reitz
2017-02-01 12:40                   ` Daniel P. Berrange [this message]
2017-02-01 12:50                     ` Max Reitz
2017-02-02  7:36                     ` Markus Armbruster
2017-02-02  7:32                   ` Markus Armbruster
2017-02-03 18:56                     ` Max Reitz
2017-02-06 10:31                       ` Daniel P. Berrange
2017-02-07 22:15                         ` Max Reitz
2017-02-08  9:19                           ` Markus Armbruster
2017-02-08 13:16                             ` Max Reitz
2017-01-26 11:04 ` [Qemu-devel] [PATCH v1 4/6] qemu-img: add support for -o " Daniel P. Berrange
2017-01-26 11:04 ` [Qemu-devel] [PATCH v1 5/6] qemu-img: introduce --target-image-opts for 'convert' command Daniel P. Berrange
2017-01-26 11:04 ` [Qemu-devel] [PATCH v1 6/6] qemu-img: copy *key-secret opts when opening newly created files Daniel P. Berrange
2017-01-30 18:39   ` Eric Blake

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=20170201124014.GH3232@redhat.com \
    --to=berrange@redhat.com \
    --cc=eblake@redhat.com \
    --cc=famz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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).