From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:34471) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QwaoJ-00027D-Pg for qemu-devel@nongnu.org; Thu, 25 Aug 2011 10:21:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QwaoE-0000z1-Vj for qemu-devel@nongnu.org; Thu, 25 Aug 2011 10:21:35 -0400 Received: from e2.ny.us.ibm.com ([32.97.182.142]:44423) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QwaoE-0000xt-QA for qemu-devel@nongnu.org; Thu, 25 Aug 2011 10:21:30 -0400 Received: from d01relay06.pok.ibm.com (d01relay06.pok.ibm.com [9.56.227.116]) by e2.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p7PDxa3C001824 for ; Thu, 25 Aug 2011 09:59:36 -0400 Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d01relay06.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p7PELHJ31163506 for ; Thu, 25 Aug 2011 10:21:18 -0400 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p7P8Kbkr005752 for ; Thu, 25 Aug 2011 02:20:37 -0600 Message-ID: <4E565A4E.2050603@us.ibm.com> Date: Thu, 25 Aug 2011 09:21:02 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1314211389-28915-1-git-send-email-aliguori@us.ibm.com> <1314211389-28915-13-git-send-email-aliguori@us.ibm.com> <20110825110957.230464dd@doriath> In-Reply-To: <20110825110957.230464dd@doriath> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 12/14] qapi: introduce change-blockdev List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Kevin Wolf , qemu-devel@nongnu.org, Michael Roth On 08/25/2011 09:09 AM, Luiz Capitulino wrote: > On Wed, 24 Aug 2011 13:43:07 -0500 > Anthony Liguori wrote: > >> A new QMP only command to change the blockdev associated with a block device. >> The semantics of change right now are just plain scary. This command introduces >> sane semantics. For more details, see the documentation in the schema file. > > IMO, this has to be split into two commits. First you introduce the new command > and then you fix do_change_block(). > > Also, there's a small problem with the generated code: it has to return -1 on > errors. The monitor expects a -1 return _and_ a qerror object on errors. The > generated code in middle mode is returning 0 even on errors, which makes QMP to > emit a UndefinedError error by default. You mean: if (local_err) { qerror_report_err(local_err); error_free(local_err); return -1; } return 0; That should behave exactly the right way as qerror_report_err() propagates sends the Error as a QError. >> + >> +## >> +# @change-blockdev: >> +# >> +# This is the preferred interface for changing a block device. >> +# >> +# @device: The block device name. >> +# >> +# @filename: The new filename for the block device. This may contain options >> +# encoded in a format specified by @format. >> +# >> +# @format: #optional The format to use open the block device > > We need a list (or a pointers) of valid formats. We do, but that needs to be a command because it depends on the build options. >> +# >> +# @password: #optional The password to use if the block device is encrypted >> +# >> +# Returns: Nothing on success. >> +# If @device is not a valid block device, DeviceNotFound >> +# If @format is not a valid block format, InvalidBlockFormat >> +# If @filename is encrypted and @password isn't supplied, >> +# DeviceEncrypted. The call should be repeated with @password >> +# supplied. >> +# If @format is not specified and @filename is a format that cannot >> +# be safely probed, MissingParameter. >> +# If @filename cannot be opened, OpenFileFailed >> +# >> +# Since: 1.0 >> +## >> +{ 'command': 'change-blockdev', >> + 'data': {'device': 'str', 'filename': 'str', '*format': 'str', >> + '*password': 'str'} } >> diff --git a/qmp-commands.hx b/qmp-commands.hx >> index c0d0ca3..cec7135 100644 >> --- a/qmp-commands.hx >> +++ b/qmp-commands.hx >> @@ -121,6 +121,14 @@ EQMP >> .mhandler.cmd_new = do_change, >> }, >> >> + { >> + .name = "change-blockdev", >> + .args_type = "device:B,target:F,arg:s?pass:s?", > > The arguments names don't match the schema. Do we need to set args_type for QMP? Regards, Anthony Liguori >> + .params = "device filename [format] [password]", >> + .help = "change a removable medium, optional format", >> + .mhandler.cmd_new = qmp_marshal_input_change_blockdev, >> + }, >> + >> SQMP >> change >> ------ >