All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luiz Capitulino <lcapitulino@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: famz@redhat.com, "Benoît Canet" <benoit@irqsave.net>,
	jcody@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org,
	stefanha@redhat.com
Subject: Re: [Qemu-devel] [PATCH V4 4/7] qmp: Allow to change password on names block driver states.
Date: Tue, 10 Dec 2013 10:16:13 -0500	[thread overview]
Message-ID: <20131210101613.7a38dadc@redhat.com> (raw)
In-Reply-To: <20131210142507.GE3656@dhcp-200-207.str.redhat.com>

On Tue, 10 Dec 2013 15:25:07 +0100
Kevin Wolf <kwolf@redhat.com> wrote:

> My objection to your approach is strong because Benoît already sent an
> alternative which I believe is less worse because with it, arguments
> actually mean what their names tell instead of having additional bools
> for "oh, and I said A, but I didn't mean it, I really want B".

Current proposal:

{ 'command': 'block_passwd', 'data': {'*device': 'str',
                                      '*node-name': 'str', 'password': 'str'} }

When I look at it, I ask myself:

 - What happens when device=NULL?

 - What happens when node-name=NULL?

 - What happens when device=NULL and node-name=NULL?

 - What happens when device != NULL and node-node != NULL?

 - What happens when device != NULL but node-node=NULL?

 - What happens when device=NULL but node-node != NULL?

My proposal:

{ 'command': 'block_passwd', 'data': {'device': 'str',
                                      '*device-is-node': 'bool', 'password': 'str'} }

 - What happens when device-is-node=NULL?

 - What happens when device-is-node != NULL?

PS: I'm not NACKing anything. My review to this series started with a
    "what about" question. I did voice my objections against this
    proposal, but didn't NACK it. Besides you're a maintainer as much
    as I am, so I could NACK this as much as you could push this
    through your tree ignoring review.

  reply	other threads:[~2013-12-10 15:46 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-05 17:14 [Qemu-devel] [PATCH V4 0/7] Giving names to BlockDriverState graph nodes Benoît Canet
2013-12-05 17:14 ` [Qemu-devel] [PATCH V4 1/7] block: Add bs->node_name to hold the name of a bs node of the bs graph Benoît Canet
2013-12-09 16:04   ` Kevin Wolf
2013-12-09 16:11   ` Kevin Wolf
2013-12-05 17:14 ` [Qemu-devel] [PATCH V4 2/7] block: Allow the user to define "node-name" option Benoît Canet
2013-12-06 15:35   ` Eric Blake
2013-12-09 16:15   ` Kevin Wolf
2013-12-11 14:42     ` Benoît Canet
2013-12-05 17:14 ` [Qemu-devel] [PATCH V4 3/7] qmp: Add a command to list the named BlockDriverState nodes Benoît Canet
2013-12-06 15:59   ` Eric Blake
2013-12-09 15:46     ` Benoît Canet
2013-12-10 15:22       ` Eric Blake
2013-12-05 17:15 ` [Qemu-devel] [PATCH V4 4/7] qmp: Allow to change password on names block driver states Benoît Canet
2013-12-06 14:27   ` Luiz Capitulino
2013-12-06 15:24     ` Eric Blake
2013-12-06 16:52       ` Luiz Capitulino
2013-12-09 13:35         ` Benoît Canet
2013-12-09 16:23         ` Kevin Wolf
2013-12-09 16:41           ` Luiz Capitulino
2013-12-09 16:48             ` Benoît Canet
2013-12-09 17:03               ` Luiz Capitulino
2013-12-09 17:16                 ` Benoît Canet
2013-12-10  9:57             ` Kevin Wolf
2013-12-10 14:06               ` Luiz Capitulino
2013-12-10 14:25                 ` Kevin Wolf
2013-12-10 15:16                   ` Luiz Capitulino [this message]
2013-12-10 15:54                     ` Kevin Wolf
2013-12-10 16:15                     ` Eric Blake
2013-12-11  3:52                     ` Fam Zheng
2013-12-11 13:20                       ` Luiz Capitulino
2013-12-05 17:15 ` [Qemu-devel] [PATCH V4 5/7] qmp: Allow block_resize to manipulate bs graph nodes Benoît Canet
2013-12-09 16:24   ` Kevin Wolf
2013-12-09 16:41     ` Benoît Canet
2013-12-10  9:59       ` Kevin Wolf
2013-12-05 17:15 ` [Qemu-devel] [PATCH V4 6/7] block: Create authorizations mechanism for external snapshots Benoît Canet
2013-12-05 17:15 ` [Qemu-devel] [PATCH V4 7/7] qmp: Allow to take external snapshots on bs graphs node Benoît Canet
2013-12-09 15:32 ` [Qemu-devel] [PATCH V4 0/7] Giving names to BlockDriverState graph nodes Kevin Wolf
2013-12-09 15:52   ` Benoît Canet

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=20131210101613.7a38dadc@redhat.com \
    --to=lcapitulino@redhat.com \
    --cc=armbru@redhat.com \
    --cc=benoit@irqsave.net \
    --cc=famz@redhat.com \
    --cc=jcody@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --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 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.