diff mbox

[05/10] qga: introduce three guest memory block commmands with stubs

Message ID 1424142892-7275-6-git-send-email-mdroth@linux.vnet.ibm.com
State New
Headers show

Commit Message

Michael Roth Feb. 17, 2015, 3:14 a.m. UTC
From: zhanghailiang <zhang.zhanghailiang@huawei.com>

Introduce three new guest commands:
guest-get-memory-blocks, guest-set-memory-blocks, guest-get-memory-block-size.

With these three commands, we can support online/offline guest's memory block
(logical memory hotplug/unplug) as required from host.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c |  38 +++++++++++++++++
 qga/commands-win32.c |  19 +++++++++
 qga/qapi-schema.json | 113 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 170 insertions(+)

Comments

Eric Blake Feb. 17, 2015, 3:26 p.m. UTC | #1
On 02/16/2015 08:14 PM, Michael Roth wrote:
> From: zhanghailiang <zhang.zhanghailiang@huawei.com>
> 
> Introduce three new guest commands:
> guest-get-memory-blocks, guest-set-memory-blocks, guest-get-memory-block-size.

Sorry for the late review, but I think guest-get-memory-block-size is
the wrong command to add.


> +##
> +# @guest-get-memory-block-size:
> +#
> +# Get the the size (in bytes) of a memory block in guest.
> +# It is the unit of memory block online/offline operation (also called Logical
> +# Memory Hotplug).
> +#
> +# Returns: memory block size in bytes.
> +#
> +# Since 2.3
> +##
> +{ 'command': 'guest-get-memory-block-size',
> +  'returns': 'int' }

Any QAPI command that returns a bare int instead of a dictionary is
non-extensible, and therefore of suspect design.  I think it would be
better to have:

{ 'command': 'guest-get-memory-block-info',
  'returns': { 'size': 'int' } }

to allow for future extension.
Michael Roth Feb. 17, 2015, 6:10 p.m. UTC | #2
Quoting Eric Blake (2015-02-17 09:26:12)
> On 02/16/2015 08:14 PM, Michael Roth wrote:
> > From: zhanghailiang <zhang.zhanghailiang@huawei.com>
> > 
> > Introduce three new guest commands:
> > guest-get-memory-blocks, guest-set-memory-blocks, guest-get-memory-block-size.
> 
> Sorry for the late review, but I think guest-get-memory-block-size is
> the wrong command to add.
> 
> 
> > +##
> > +# @guest-get-memory-block-size:
> > +#
> > +# Get the the size (in bytes) of a memory block in guest.
> > +# It is the unit of memory block online/offline operation (also called Logical
> > +# Memory Hotplug).
> > +#
> > +# Returns: memory block size in bytes.
> > +#
> > +# Since 2.3
> > +##
> > +{ 'command': 'guest-get-memory-block-size',
> > +  'returns': 'int' }
> 
> Any QAPI command that returns a bare int instead of a dictionary is
> non-extensible, and therefore of suspect design.  I think it would be
> better to have:
> 
> { 'command': 'guest-get-memory-block-info',
>   'returns': { 'size': 'int' } }
> 
> to allow for future extension.

It seems like a reasonable suggestion to me. I can change it in my
tree if there are no objections. zhanghailiang?

> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
Zhanghailiang Feb. 25, 2015, 2:46 a.m. UTC | #3
On 2015/2/18 2:10, Michael Roth wrote:
> Quoting Eric Blake (2015-02-17 09:26:12)
>> On 02/16/2015 08:14 PM, Michael Roth wrote:
>>> From: zhanghailiang <zhang.zhanghailiang@huawei.com>
>>>
>>> Introduce three new guest commands:
>>> guest-get-memory-blocks, guest-set-memory-blocks, guest-get-memory-block-size.
>>
>> Sorry for the late review, but I think guest-get-memory-block-size is
>> the wrong command to add.
>>
>>
>>> +##
>>> +# @guest-get-memory-block-size:
>>> +#
>>> +# Get the the size (in bytes) of a memory block in guest.
>>> +# It is the unit of memory block online/offline operation (also called Logical
>>> +# Memory Hotplug).
>>> +#
>>> +# Returns: memory block size in bytes.
>>> +#
>>> +# Since 2.3
>>> +##
>>> +{ 'command': 'guest-get-memory-block-size',
>>> +  'returns': 'int' }
>>
>> Any QAPI command that returns a bare int instead of a dictionary is
>> non-extensible, and therefore of suspect design.  I think it would be
>> better to have:
>>
>> { 'command': 'guest-get-memory-block-info',
>>    'returns': { 'size': 'int' } }
>>
>> to allow for future extension.
>
> It seems like a reasonable suggestion to me. I can change it in my
> tree if there are no objections. zhanghailiang?
>

OK, you are free to do that, please. Thanks ;)

>>
>> --
>> Eric Blake   eblake redhat com    +1-919-301-3266
>> Libvirt virtualization library http://libvirt.org
>
>
>
Zhanghailiang Feb. 25, 2015, 2:51 a.m. UTC | #4
On 2015/2/17 23:26, Eric Blake wrote:
> On 02/16/2015 08:14 PM, Michael Roth wrote:
>> From: zhanghailiang <zhang.zhanghailiang@huawei.com>
>>
>> Introduce three new guest commands:
>> guest-get-memory-blocks, guest-set-memory-blocks, guest-get-memory-block-size.
>
> Sorry for the late review, but I think guest-get-memory-block-size is
> the wrong command to add.
>
>
>> +##
>> +# @guest-get-memory-block-size:
>> +#
>> +# Get the the size (in bytes) of a memory block in guest.
>> +# It is the unit of memory block online/offline operation (also called Logical
>> +# Memory Hotplug).
>> +#
>> +# Returns: memory block size in bytes.
>> +#
>> +# Since 2.3
>> +##
>> +{ 'command': 'guest-get-memory-block-size',
>> +  'returns': 'int' }
>
> Any QAPI command that returns a bare int instead of a dictionary is
> non-extensible, and therefore of suspect design.  I think it would be
> better to have:
>
> { 'command': 'guest-get-memory-block-info',
>    'returns': { 'size': 'int' } }
>
> to allow for future extension.
>

Good idea, and Michael Roth has changed it like that, thanks for you comments.
diff mbox

Patch

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index ed527a3..0ce27a4 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1992,6 +1992,25 @@  out:
     }
 }
 
+GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
+GuestMemoryBlockResponseList *
+qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks, Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
+int64_t qmp_guest_get_memory_block_size(Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return -1;
+}
+
 #else /* defined(__linux__) */
 
 void qmp_guest_suspend_disk(Error **errp)
@@ -2035,6 +2054,25 @@  void qmp_guest_set_user_password(const char *username,
     error_set(errp, QERR_UNSUPPORTED);
 }
 
+GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
+GuestMemoryBlockResponseList *
+qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks, Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
+int64_t qmp_guest_get_memory_block_size(Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return -1;
+}
+
 #endif
 
 #if !defined(CONFIG_FSFREEZE)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 9af7950..30e377b 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -684,6 +684,25 @@  void qmp_guest_set_user_password(const char *username,
     error_set(errp, QERR_UNSUPPORTED);
 }
 
+GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
+GuestMemoryBlockResponseList *
+qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks, Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
+int64_t qmp_guest_get_memory_block_size(Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return -1;
+}
+
 /* add unsupported commands to the blacklist */
 GList *ga_command_blacklist_init(GList *blacklist)
 {
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 5117b65..b69cfe1 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -765,3 +765,116 @@ 
 ##
 { 'command': 'guest-set-user-password',
   'data': { 'username': 'str', 'password': 'str', 'crypted': 'bool' } }
+
+# @GuestMemoryBlock:
+#
+# @phys-index: Arbitrary guest-specific unique identifier of the MEMORY BLOCK.
+#
+# @online: Whether the MEMORY BLOCK is enabled in guest.
+#
+# @can-offline: #optional Whether offlining the MEMORY BLOCK is possible.
+#               This member is always filled in by the guest agent when the
+#               structure is returned, and always ignored on input (hence it
+#               can be omitted then).
+#
+# Since: 2.3
+##
+{ 'type': 'GuestMemoryBlock',
+  'data': {'phys-index': 'uint64',
+           'online': 'bool',
+           '*can-offline': 'bool'} }
+
+##
+# @guest-get-memory-blocks:
+#
+# Retrieve the list of the guest's memory blocks.
+#
+# This is a read-only operation.
+#
+# Returns: The list of all memory blocks the guest knows about.
+# Each memory block is put on the list exactly once, but their order
+# is unspecified.
+#
+# Since: 2.3
+##
+{ 'command': 'guest-get-memory-blocks',
+  'returns': ['GuestMemoryBlock'] }
+
+##
+# @GuestMemoryBlockResponseType
+#
+# An enumeration of memory block operation result.
+#
+# @sucess: the operation of online/offline memory block is successful.
+# @not-found: can't find the corresponding memoryXXX directory in sysfs.
+# @operation-not-supported: for some old kernels, it does not support
+#                           online or offline memory block.
+# @operation-failed: the operation of online/offline memory block fails,
+#                    because of some errors happen.
+#
+# Since: 2.3
+##
+{ 'enum': 'GuestMemoryBlockResponseType',
+  'data': ['success', 'not-found', 'operation-not-supported',
+           'operation-failed'] }
+
+##
+# @GuestMemoryBlockResponse:
+#
+# @phys-index: same with the 'phys-index' member of @GuestMemoryBlock.
+#
+# @response: the result of memory block operation.
+#
+# @error-code: #optional the error number.
+#               When memory block operation fails, we assign the value of
+#               'errno' to this member, it indicates what goes wrong.
+#               When the operation succeeds, it will be omitted.
+#
+# Since: 2.3
+##
+{ 'type': 'GuestMemoryBlockResponse',
+  'data': { 'phys-index': 'uint64',
+            'response': 'GuestMemoryBlockResponseType',
+            '*error-code': 'int' }}
+
+##
+# @guest-set-memory-blocks:
+#
+# Attempt to reconfigure (currently: enable/disable) state of memory blocks
+# inside the guest.
+#
+# The input list is processed node by node in order. In each node @phys-index
+# is used to look up the guest MEMORY BLOCK, for which @online specifies the
+# requested state. The set of distinct @phys-index's is only required to be a
+# subset of the guest-supported identifiers. There's no restriction on list
+# length or on repeating the same @phys-index (with possibly different @online
+# field).
+# Preferably the input list should describe a modified subset of
+# @guest-get-memory-blocks' return value.
+#
+# Returns: The operation results, it is a list of @GuestMemoryBlockResponse,
+#          which is corresponding to the input list.
+#
+#          Note: it will return NULL if the @mem-blks list was empty on input,
+#          or there is an error, and in this case, guest state will not be
+#          changed.
+#
+# Since: 2.3
+##
+{ 'command': 'guest-set-memory-blocks',
+  'data':    {'mem-blks': ['GuestMemoryBlock'] },
+  'returns': ['GuestMemoryBlockResponse'] }
+
+##
+# @guest-get-memory-block-size:
+#
+# Get the the size (in bytes) of a memory block in guest.
+# It is the unit of memory block online/offline operation (also called Logical
+# Memory Hotplug).
+#
+# Returns: memory block size in bytes.
+#
+# Since 2.3
+##
+{ 'command': 'guest-get-memory-block-size',
+  'returns': 'int' }