qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Lieven <pl@kamp.de>
To: Kevin Wolf <kwolf@redhat.com>
Cc: Eric Blake <eblake@redhat.com>,
	Ashijeet Acharya <ashijeetacharya@gmail.com>,
	jcody@redhat.com, mreitz@redhat.com, armbru@redhat.com,
	qemu-devel@nongnu.org, qemu-block@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v6 0/2] allow blockdev-add for NFS
Date: Thu, 19 Jan 2017 18:13:26 +0100	[thread overview]
Message-ID: <ad3b4e21-070a-ada6-580c-aef9284df1dd@kamp.de> (raw)
In-Reply-To: <20170119170830.GD5443@noname.redhat.com>

Am 19.01.2017 um 18:08 schrieb Kevin Wolf:
> Am 19.01.2017 um 16:58 hat Peter Lieven geschrieben:
>> Am 19.01.2017 um 16:55 schrieb Kevin Wolf:
>>> Am 19.01.2017 um 16:44 hat Peter Lieven geschrieben:
>>>> Am 19.01.2017 um 16:42 schrieb Kevin Wolf:
>>>>> Am 19.01.2017 um 16:34 hat Peter Lieven geschrieben:
>>>>>> Am 19.01.2017 um 16:20 schrieb Kevin Wolf:
>>>>>>> Am 19.01.2017 um 15:59 hat Eric Blake geschrieben:
>>>>>>>> On 01/19/2017 08:30 AM, Peter Lieven wrote:
>>>>>>>>>>> qemu-img: Could not open
>>>>>>>>>>> 'nfs://172.21.200.61/vcore-dev-cdrom/templates/VC_debian8-20170116.qcow2,linux?readahead=131072':
>>>>>>>>>>> Block protocol 'nfs' doesn't support the option 'readahead-size'
>>>>>>>>>>>
>>>>>>>>>>> Please let me know if the below fix would be correct:
>>>>>>>>>> No, this needs to be fixed the other way round: runtime_opts must use
>>>>>>>>>> the names as specified in the schema, and nfs_client_open() must access
>>>>>>>>>> them as such. Without that, blockdev-add can't work (and the command
>>>>>>>>>> line only with the "wrong" old option names from the URL, whereas it
>>>>>>>>>> should be using the same names as the QAPI schema).
>>>>>>>>> Shouldn't we support both for backwards compatiblity.?
>>>>>>>> blockdev-add only needs to support the modern naming.  But yes,
>>>>>>>> preserving back-compat spelling of the command-line spellings, as well
>>>>>>>> as matching blockdev-add spellings, is desirable.
>>>>>>> We only just added the individual command line options, previously it
>>>>>>> only supported the URL.
>>>>>>>
>>>>>>> It's true that we have the messed up version of the options in 2.8, so
>>>>>>> strictly speaking we would break compatibility with a release, but it's
>>>>>>> only one release, it's only the nfs driver, and the documentation of the
>>>>>>> options is the schema, which had the right option names even in 2.8
>>>>>>> (they just didn't work).
>>>>>>>
>>>>>>> So I wouldn't feel bad about removing the wrong names in this specific
>>>>>>> case.
>>>>>> So want exactly do you want to do? Fix the names in the QAPI schema
>>>>>> to use the old naming?
>>>>> No, fix the command line to use the names in the QAPI schema.
>>>>>
>>>>> The option names from the URL were never supposed to be supported on the
>>>>> command line.
>>>> Okay, so no backwards compatiblity? I actually used the options on the command line...
>>> Well, do you _need_ compatibility?
>>>
>>> It can certainly be done, but as the (wrong) options on the command line
>>> have only existed since November and were never documented, I wouldn't
>>> bother unless there's a good reason.
>> Every Qemu before 2.8.0 was working with sth like:
>>
>> qemu -cdrom nfs://10.0.0.1/expory/my.iso?readahead=131072
> That will keep working. We're not changing the URL parsing, just the
> runtime_opts and its accesses in nfs_client_open(). The translation in
> nfs_parse_uri() stays intact with the fixes.
>
> What will stop working (and only worked in 2.8.0) is this:
>
>      qemu -drive media=cdrom,driver=nfs,server.host=10.0.0.1,path=export/my.iso,readahead=131072
>
> Also, I think the fixes should be Cc: qemu-stable, so that 2.8.1 will
> work correctly again.

Okay, I hope I got it. Will send a patch tomorrow.

Peter

  reply	other threads:[~2017-01-19 17:13 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-31 15:05 [Qemu-devel] [PATCH v6 0/2] allow blockdev-add for NFS Ashijeet Acharya
2016-10-31 15:05 ` [Qemu-devel] [PATCH v6 1/2] block/nfs: Introduce runtime_opts in NFS Ashijeet Acharya
2016-10-31 15:48   ` Kevin Wolf
2016-10-31 15:05 ` [Qemu-devel] [PATCH v6 2/2] qapi: allow blockdev-add for NFS Ashijeet Acharya
2016-10-31 17:20 ` [Qemu-devel] [PATCH v6 0/2] " Kevin Wolf
2017-01-17 15:14   ` Peter Lieven
2017-01-18  9:59     ` Kevin Wolf
2017-01-19 14:30       ` Peter Lieven
2017-01-19 14:59         ` Eric Blake
2017-01-19 15:20           ` Kevin Wolf
2017-01-19 15:34             ` Peter Lieven
2017-01-19 15:42               ` Kevin Wolf
2017-01-19 15:44                 ` Peter Lieven
2017-01-19 15:55                   ` Kevin Wolf
2017-01-19 15:58                     ` Peter Lieven
2017-01-19 17:08                       ` Kevin Wolf
2017-01-19 17:13                         ` Peter Lieven [this message]
2017-01-19 15:47                 ` Peter Lieven

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=ad3b4e21-070a-ada6-580c-aef9284df1dd@kamp.de \
    --to=pl@kamp.de \
    --cc=armbru@redhat.com \
    --cc=ashijeetacharya@gmail.com \
    --cc=eblake@redhat.com \
    --cc=jcody@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).