All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: Lucas Meneghel Rodrigues <lmr@redhat.com>,
	Stefan Hajnoczi <stefanha@gmail.com>,
	qemu-devel@nongnu.org,
	Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>,
	Juan Quintela <quintela@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 3/4] qcow2: add a migration blocker
Date: Mon, 14 Nov 2011 17:37:07 +0100	[thread overview]
Message-ID: <4EC143B3.4090303@redhat.com> (raw)
In-Reply-To: <4EC140E3.1070001@codemonkey.ws>

Am 14.11.2011 17:25, schrieb Anthony Liguori:
> On 11/14/2011 09:51 AM, Stefan Hajnoczi wrote:
>> On Fri, Nov 11, 2011 at 8:29 PM, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>>> +#define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
>>> +    "{ 'class': 'BlockFormatFeatureNotSupported', 'data': { 'format': %s, 'name': %s, 'feature': %s } }"
>>> +
>>
>> Isn't having a separate error going to make life harder for management
>> tool writers?  I would have expected one "migration not supported"
>> error, regardless of whether the reason is ivshmem, qcow2, or anything
>> else.
> 
> Errors shouldn't be tied to verbs.  IOW, if you have a migrate command, you 
> don't want to have a MigrationFailed error because that's tied to a specific verb.
> 
> Instead, you want the errors to provide additional information about the verb 
> failed.  In this case, the verb is failing because you're requesting to use a 
> feature that is not supported by this particular block format.

We already have QERR_UNKNOWN_BLOCK_FORMAT_FEATURE which is very similar
(in fact, FEATURE_NOT_SUPPORTED is a much better name which we should
have used from the beginning).

"unknown feature" doesn't really fit here and it's too late to change
the name of the existing error. We'll probably have to live with two
different error messages.

Kevin

  reply	other threads:[~2011-11-14 16:34 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-11 20:29 [Qemu-devel] [PATCH 1/4] migrate: add migration blockers Anthony Liguori
2011-11-11 20:29 ` [Qemu-devel] [PATCH 2/4] ivshmem: use migration blockers to prevent live migration in peer mode Anthony Liguori
2011-11-12 20:24   ` Aneesh Kumar K.V
2011-11-11 20:29 ` [Qemu-devel] [PATCH 3/4] qcow2: add a migration blocker Anthony Liguori
2011-11-14 15:51   ` Stefan Hajnoczi
2011-11-14 16:25     ` Anthony Liguori
2011-11-14 16:37       ` Kevin Wolf [this message]
2011-11-14 16:45       ` Stefan Hajnoczi
2011-11-14 16:49         ` Anthony Liguori
2011-11-14 16:57           ` Stefan Hajnoczi
2011-11-11 20:29 ` [Qemu-devel] [PATCH 4/4] qed: add " Anthony Liguori

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=4EC143B3.4090303@redhat.com \
    --to=kwolf@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=lmr@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=stefanha@gmail.com \
    --cc=stefanha@linux.vnet.ibm.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.