All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Max Reitz <mreitz@redhat.com>
Cc: "Daniel P. Berrange" <berrange@redhat.com>,
	Kevin Wolf <kwolf@redhat.com>, Fam Zheng <famz@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: Thu, 02 Feb 2017 08:32:23 +0100	[thread overview]
Message-ID: <87bmul9hfc.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <37f445c0-c174-bca4-7073-7aa6c64046b0@redhat.com> (Max Reitz's message of "Wed, 1 Feb 2017 13:31:01 +0100")

Max Reitz <mreitz@redhat.com> writes:

> 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".

No.  /bin/dd treats exactly block special files as block special files,
so the qemu-img command that tries to behave like it should do, too.

In case you say that's inconvenient: pretty much everything about dd's
archaic user interface is inconvenient.  If you want convenient, roll
your own.  If you want familiar, stick to the original.

  parent reply	other threads:[~2017-02-02  7:32 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
2017-02-01 12:50                     ` Max Reitz
2017-02-02  7:36                     ` Markus Armbruster
2017-02-02  7:32                   ` Markus Armbruster [this message]
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=87bmul9hfc.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=berrange@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 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.