Re: [Qemu-devel] [RFC 05/10] QMP: Introduce the blockdev-tray-open command
On (Mon) 06 Jun 2011 [11:38:03], Luiz Capitulino wrote:
On Mon, 6 Jun 2011 17:10:32 +0530
Amit Shah amit.s...@redhat.com wrote:
On (Fri) 03 Jun 2011 [16:03:57], Luiz Capitulino wrote:
+static int tray_open(const char *device, int remove, int force)
+{
+BlockDriverState *bs;
+
+bs = bdrv_removable_find(device);
+if (!bs) {
+return -1;
+}
+
+if (bdrv_eject(bs, 1, force) 0) {
+/* FIXME: will report undefined error in QMP */
+return -1;
+}
+
+if (remove) {
+bdrv_close(bs);
+}
+
+return 0;
+}
What's the reason to tie the 'remove' with tray open?
In my first try I had a command called 'blockdev-media-remove', but then
I had the impression that I was going too far as the only reason a client
would ever want to open the tray is to remove the media.
Not necessary -- CD/DVD writers eject and reload trays after erasing
media -- at least they used to.
Won't it be
simpler to have it separated out, perhaps a 'change' event instead of
'insert' that can accept NULL which means just remove medium?
You meant 'command' instead of 'event', right?
I don't think a change command makes sense, because it's just a shortcut
to open/remove/insert/close.
Yes, command, sorry.
And by 'change' I don't mean the current monitor change command --
that's a badly-named one.
By change I mean just that -- replace the media. And that should
succeed only if tray is open. And tray remains open after the change.
Amit
Re: [Qemu-devel] [RFC 05/10] QMP: Introduce the blockdev-tray-open command
On Tue, 7 Jun 2011 18:59:12 +0530
Amit Shah amit.s...@redhat.com wrote:
On (Mon) 06 Jun 2011 [11:38:03], Luiz Capitulino wrote:
On Mon, 6 Jun 2011 17:10:32 +0530
Amit Shah amit.s...@redhat.com wrote:
On (Fri) 03 Jun 2011 [16:03:57], Luiz Capitulino wrote:
+static int tray_open(const char *device, int remove, int force)
+{
+BlockDriverState *bs;
+
+bs = bdrv_removable_find(device);
+if (!bs) {
+return -1;
+}
+
+if (bdrv_eject(bs, 1, force) 0) {
+/* FIXME: will report undefined error in QMP */
+return -1;
+}
+
+if (remove) {
+bdrv_close(bs);
+}
+
+return 0;
+}
What's the reason to tie the 'remove' with tray open?
In my first try I had a command called 'blockdev-media-remove', but then
I had the impression that I was going too far as the only reason a client
would ever want to open the tray is to remove the media.
Not necessary -- CD/DVD writers eject and reload trays after erasing
media -- at least they used to.
But that's not done by the VM's user/client.
I think I'll add it anyway, as it's what people seem to expect from an
API like this.
Won't it be
simpler to have it separated out, perhaps a 'change' event instead of
'insert' that can accept NULL which means just remove medium?
You meant 'command' instead of 'event', right?
I don't think a change command makes sense, because it's just a shortcut
to open/remove/insert/close.
Yes, command, sorry.
And by 'change' I don't mean the current monitor change command --
that's a badly-named one.
By change I mean just that -- replace the media. And that should
succeed only if tray is open. And tray remains open after the change.
The same thing can be done with blockdev-media-remove and
blockdev-media-insert, so I don't think this adds value.
Re: [Qemu-devel] [RFC 05/10] QMP: Introduce the blockdev-tray-open command
On (Fri) 03 Jun 2011 [16:03:57], Luiz Capitulino wrote:
+static int tray_open(const char *device, int remove, int force)
+{
+BlockDriverState *bs;
+
+bs = bdrv_removable_find(device);
+if (!bs) {
+return -1;
+}
+
+if (bdrv_eject(bs, 1, force) 0) {
+/* FIXME: will report undefined error in QMP */
+return -1;
+}
+
+if (remove) {
+bdrv_close(bs);
+}
+
+return 0;
+}
What's the reason to tie the 'remove' with tray open? Won't it be
simpler to have it separated out, perhaps a 'change' event instead of
'insert' that can accept NULL which means just remove medium?
Amit
Re: [Qemu-devel] [RFC 05/10] QMP: Introduce the blockdev-tray-open command
Luiz Capitulino lcapitul...@redhat.com writes: This command opens a removable media drive's tray. It's only available in QMP. The do_tray_open() function is split into two smaller functions because next commits will also use them. Please, check the command's documentation (being introduced in this commit) for a detailed description. XXX: Should we return an error if the tray is already open? Use case? For what it's worth, Linux ioctl CDROMCLOSETRAY appears to return success when it does nothing.
Re: [Qemu-devel] [RFC 05/10] QMP: Introduce the blockdev-tray-open command
On Mon, 6 Jun 2011 17:10:32 +0530
Amit Shah amit.s...@redhat.com wrote:
On (Fri) 03 Jun 2011 [16:03:57], Luiz Capitulino wrote:
+static int tray_open(const char *device, int remove, int force)
+{
+BlockDriverState *bs;
+
+bs = bdrv_removable_find(device);
+if (!bs) {
+return -1;
+}
+
+if (bdrv_eject(bs, 1, force) 0) {
+/* FIXME: will report undefined error in QMP */
+return -1;
+}
+
+if (remove) {
+bdrv_close(bs);
+}
+
+return 0;
+}
What's the reason to tie the 'remove' with tray open?
In my first try I had a command called 'blockdev-media-remove', but then
I had the impression that I was going too far as the only reason a client
would ever want to open the tray is to remove the media.
Won't it be
simpler to have it separated out, perhaps a 'change' event instead of
'insert' that can accept NULL which means just remove medium?
You meant 'command' instead of 'event', right?
I don't think a change command makes sense, because it's just a shortcut
to open/remove/insert/close.
[Qemu-devel] [RFC 05/10] QMP: Introduce the blockdev-tray-open command
This command opens a removable media drive's tray. It's only available
in QMP.
The do_tray_open() function is split into two smaller functions because
next commits will also use them.
Please, check the command's documentation (being introduced in this
commit) for a detailed description.
XXX: Should we return an error if the tray is already open?
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
blockdev.c | 46 ++
blockdev.h |1 +
qmp-commands.hx | 27 +++
3 files changed, 74 insertions(+), 0 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 6e0eb83..b1c705c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -665,6 +665,52 @@ static int eject_device(Monitor *mon, BlockDriverState
*bs, int force)
return 0;
}
+static BlockDriverState *bdrv_removable_find(const char *name)
+{
+BlockDriverState *bs;
+
+bs = bdrv_find(name);
+if (!bs) {
+qerror_report(QERR_DEVICE_NOT_FOUND, name);
+return NULL;
+}
+
+if (!bdrv_is_removable(bs)) {
+qerror_report(QERR_DEVICE_NOT_REMOVABLE, bdrv_get_device_name(bs));
+return NULL;
+}
+
+return bs;
+}
+
+static int tray_open(const char *device, int remove, int force)
+{
+BlockDriverState *bs;
+
+bs = bdrv_removable_find(device);
+if (!bs) {
+return -1;
+}
+
+if (bdrv_eject(bs, 1, force) 0) {
+/* FIXME: will report undefined error in QMP */
+return -1;
+}
+
+if (remove) {
+bdrv_close(bs);
+}
+
+return 0;
+}
+
+int do_tray_open(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+return tray_open(qdict_get_str(qdict, device),
+ qdict_get_try_bool(qdict, remove, 0),
+ qdict_get_try_bool(qdict, force, 0));
+}
+
int do_eject(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
BlockDriverState *bs;
diff --git a/blockdev.h b/blockdev.h
index 3587786..5e46aae 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -65,5 +65,6 @@ int do_change_block(Monitor *mon, const char *device,
int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data);
int do_block_resize(Monitor *mon, const QDict *qdict, QObject **ret_data);
+int do_tray_open(Monitor *mon, const QDict *qdict, QObject **ret_data);
#endif
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 8393f22..58ab132 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -153,6 +153,33 @@ Examples:
EQMP
{
+.name = blockdev-tray-open,
+.args_type = device:B,force:-f,remove:-r,
+.mhandler.cmd_new = do_tray_open,
+},
+
+SQMP
+blockdev-tray-open
+--
+
+Open a removable media drive's tray.
+
+Arguments:
+
+- device: device name (json-string)
+- force: force ejection (json-bool, optional)
+- remove: remove the media (json-bool, optional)
+
+Example:
+
+- { execute: blockdev-tray-open, arguments: { device: ide1-cd0 } }
+- { return: {} }
+
+Note: The tray remains open after this command is issued
+
+EQMP
+
+{
.name = screendump,
.args_type = filename:F,
.params = filename,
--
1.7.4.4
