Message ID | 1422387983-32153-48-git-send-email-mreitz@redhat.com |
---|---|
State | New |
Headers | show |
On 01/27/2015 12:46 PM, Max Reitz wrote: > Add an option to qmp_blockdev_change_medium() which allows changing the > read-only status of the block device whose medium is changed. > > Some drives do not have a inherently fixed read-only status; for > instance, floppy disks can be set read-only or writable independently of > the drive. Some users may find it useful to be able to therefore change > the read-only status of a block device when changing the medium. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > blockdev.c | 25 ++++++++++++++++++++++++- > hmp.c | 2 +- > qapi/block-core.json | 24 +++++++++++++++++++++++- > qmp-commands.hx | 24 +++++++++++++++++++++++- > qmp.c | 3 ++- > 5 files changed, 73 insertions(+), 5 deletions(-) > > > ## > +# @BlockdevChangeReadOnlyMode: > +# > +# Specifies the new read-only mode of a block device subject to the > +# @blockdev-change-medium command. > +# > +# @retain: Retains the current read-only mode > +# > +# @ro: Makes the device read-only > +# > +# @rw: Makes the device writable > +# > +# Since: 2.3 > +## > +{ 'enum': 'BlockdevChangeReadOnlyMode', > + 'data': ['retain', 'ro', 'rw'] } Bike-shedding; would 'read-only' and 'read-write' look any better than abbreviations? Doesn't affect functionality, though. > + > + > +## > # @blockdev-change-medium: > # > # Changes the medium inserted into a block device by ejecting the current medium > @@ -1799,12 +1817,16 @@ > # @format: #optional, format to open the new image with (defaults to the > # probed format) > # > +# @read-only: #optional, change the read-only mode of the device; defaults to > +# 'retain' "read-only":"rw" looks weird. Maybe naming this "read-mode" instead of "read-only" would help. Again, bikeshedding that doesn't affect functionality, but worth considering for the interface cleanliness. So functionally, if nothing changes, you can add: Reviewed-by: Eric Blake <eblake@redhat.com> But if you change the interface on a respin, drop my R-b to make sure I check and still like the new naming conventions.
On 2015-01-28 at 16:08, Eric Blake wrote: > On 01/27/2015 12:46 PM, Max Reitz wrote: >> Add an option to qmp_blockdev_change_medium() which allows changing the >> read-only status of the block device whose medium is changed. >> >> Some drives do not have a inherently fixed read-only status; for >> instance, floppy disks can be set read-only or writable independently of >> the drive. Some users may find it useful to be able to therefore change >> the read-only status of a block device when changing the medium. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> blockdev.c | 25 ++++++++++++++++++++++++- >> hmp.c | 2 +- >> qapi/block-core.json | 24 +++++++++++++++++++++++- >> qmp-commands.hx | 24 +++++++++++++++++++++++- >> qmp.c | 3 ++- >> 5 files changed, 73 insertions(+), 5 deletions(-) >> >> >> ## >> +# @BlockdevChangeReadOnlyMode: >> +# >> +# Specifies the new read-only mode of a block device subject to the >> +# @blockdev-change-medium command. >> +# >> +# @retain: Retains the current read-only mode >> +# >> +# @ro: Makes the device read-only >> +# >> +# @rw: Makes the device writable >> +# >> +# Since: 2.3 >> +## >> +{ 'enum': 'BlockdevChangeReadOnlyMode', >> + 'data': ['retain', 'ro', 'rw'] } > Bike-shedding; would 'read-only' and 'read-write' look any better than > abbreviations? Doesn't affect functionality, though. I don't mind either way. >> + >> + >> +## >> # @blockdev-change-medium: >> # >> # Changes the medium inserted into a block device by ejecting the current medium >> @@ -1799,12 +1817,16 @@ >> # @format: #optional, format to open the new image with (defaults to the >> # probed format) >> # >> +# @read-only: #optional, change the read-only mode of the device; defaults to >> +# 'retain' > "read-only":"rw" looks weird. Maybe naming this "read-mode" instead of > "read-only" would help. Again, bikeshedding that doesn't affect > functionality, but worth considering for the interface cleanliness. Well, actually it's write-mode, because reading will always be possible. :-) "access" would be another possibility, or "read-only-mode". > So functionally, if nothing changes, you can add: > Reviewed-by: Eric Blake <eblake@redhat.com> > > But if you change the interface on a respin, drop my R-b to make sure I > check and still like the new naming conventions. Understood. Max
diff --git a/blockdev.c b/blockdev.c index 2ada2b1..8ed2fec 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2014,6 +2014,8 @@ void qmp_blockdev_insert_medium(const char *device, const char *node_name, void qmp_blockdev_change_medium(const char *device, const char *filename, bool has_format, const char *format, + bool has_read_only, + BlockdevChangeReadOnlyMode read_only, Error **errp) { BlockBackend *blk; @@ -2034,7 +2036,28 @@ void qmp_blockdev_change_medium(const char *device, const char *filename, } blk_rs = blk_get_root_state(blk); - bdrv_flags = blk_rs->read_only ? 0 : BDRV_O_RDWR; + + if (!has_read_only) { + read_only = BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN; + } + + switch (read_only) { + case BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN: + bdrv_flags = blk_rs->read_only ? 0 : BDRV_O_RDWR; + break; + + case BLOCKDEV_CHANGE_READ_ONLY_MODE_RO: + bdrv_flags = 0; + break; + + case BLOCKDEV_CHANGE_READ_ONLY_MODE_RW: + bdrv_flags = BDRV_O_RDWR; + break; + + default: + abort(); + } + bdrv_flags |= blk_rs->open_flags & ~BDRV_O_RDWR; if (has_format) { diff --git a/hmp.c b/hmp.c index 300e7d8..dbf0947 100644 --- a/hmp.c +++ b/hmp.c @@ -1190,7 +1190,7 @@ void hmp_change(Monitor *mon, const QDict *qdict) } qmp_change("vnc", target, !!arg, arg, &err); } else { - qmp_blockdev_change_medium(device, target, !!arg, arg, &err); + qmp_blockdev_change_medium(device, target, !!arg, arg, false, 0, &err); if (err && error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) { error_free(err); diff --git a/qapi/block-core.json b/qapi/block-core.json index d3c3ca7..eb2724e 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1785,6 +1785,24 @@ ## +# @BlockdevChangeReadOnlyMode: +# +# Specifies the new read-only mode of a block device subject to the +# @blockdev-change-medium command. +# +# @retain: Retains the current read-only mode +# +# @ro: Makes the device read-only +# +# @rw: Makes the device writable +# +# Since: 2.3 +## +{ 'enum': 'BlockdevChangeReadOnlyMode', + 'data': ['retain', 'ro', 'rw'] } + + +## # @blockdev-change-medium: # # Changes the medium inserted into a block device by ejecting the current medium @@ -1799,12 +1817,16 @@ # @format: #optional, format to open the new image with (defaults to the # probed format) # +# @read-only: #optional, change the read-only mode of the device; defaults to +# 'retain' +# # Since: 2.3 ## { 'command': 'blockdev-change-medium', 'data': { 'device': 'str', 'filename': 'str', - '*format': 'str' } } + '*format': 'str', + '*read-only': 'BlockdevChangeReadOnlyMode' } } ## diff --git a/qmp-commands.hx b/qmp-commands.hx index 1987a09..f14953a 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3855,7 +3855,7 @@ EQMP { .name = "blockdev-change-medium", - .args_type = "device:B,filename:F,format:s?", + .args_type = "device:B,filename:F,format:s?,read-only:s?", .mhandler.cmd_new = qmp_marshal_input_blockdev_change_medium, }, @@ -3871,6 +3871,8 @@ Arguments: - "device": device name (json-string) - "filename": filename of the new image (json-string) - "format": format of the new image (json-string, optional) +- "read-only": new read-only mode (json-string, optional) + - Possible values: "retain" (default), "ro", "rw" Examples: @@ -3882,6 +3884,26 @@ Examples: "format": "raw" } } <- { "return": {} } +2. Load a read-only medium into a writable drive + +-> { "execute": "blockdev-change-medium", + "arguments": { "device": "isa-fd0", + "filename": "/srv/images/ro.img", + "format": "raw", + "read-only": "retain" } } + +<- { "error": + { "class": "GenericError", + "desc": "Could not open '/srv/images/ro.img': Permission denied" } } + +-> { "execute": "blockdev-change-medium", + "arguments": { "device": "isa-fd0", + "filename": "/srv/images/ro.img", + "format": "raw", + "read-only": "ro" } } + +<- { "return": {} } + EQMP { diff --git a/qmp.c b/qmp.c index 1cfdb74..d414118 100644 --- a/qmp.c +++ b/qmp.c @@ -417,7 +417,8 @@ void qmp_change(const char *device, const char *target, if (strcmp(device, "vnc") == 0) { qmp_change_vnc(target, has_arg, arg, errp); } else { - qmp_blockdev_change_medium(device, target, has_arg, arg, errp); + qmp_blockdev_change_medium(device, target, has_arg, arg, false, 0, + errp); } }
Add an option to qmp_blockdev_change_medium() which allows changing the read-only status of the block device whose medium is changed. Some drives do not have a inherently fixed read-only status; for instance, floppy disks can be set read-only or writable independently of the drive. Some users may find it useful to be able to therefore change the read-only status of a block device when changing the medium. Signed-off-by: Max Reitz <mreitz@redhat.com> --- blockdev.c | 25 ++++++++++++++++++++++++- hmp.c | 2 +- qapi/block-core.json | 24 +++++++++++++++++++++++- qmp-commands.hx | 24 +++++++++++++++++++++++- qmp.c | 3 ++- 5 files changed, 73 insertions(+), 5 deletions(-)