Re: [Qemu-devel] [PATCH COLO-Frame v8 21/34] COLO: Add new command parameter 'forward_nic' 'colo_script' for net

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2363 bytes --]

On 07/29/2015 02:45 AM, zhanghailiang wrote:
> The 'forward_nic' should be assigned with network name,
> for exmple, 'eth2'. It will be parameter of 'colo_script',

s/exmple/example/

> 'colo_script' should be assigned with an scirpt path.

s/scirpt/script/

> 
> We parse these parameter in tap.

s/parameter/parameters/

> 
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> ---
>  include/net/colo-nic.h | 23 +++++++++++++++++++++++
>  include/net/net.h      |  2 ++
>  net/tap.c              | 26 +++++++++++++++++++++++---
>  qapi-schema.json       |  8 +++++++-
>  qemu-options.hx        |  7 +++++++
>  5 files changed, 62 insertions(+), 4 deletions(-)
>  create mode 100644 include/net/colo-nic.h

Interface review.


> +
> +typedef struct COLONicState {
> +    char nicname[128]; /* forward dev */
> +    char script[1024]; /* colo script */
> +    char ifname[128];  /* e.g. tap name */
> +} COLONicState;

Are these fixed-width fields sufficiently sized; and are you sure you
won't overflow the buffers?


> +++ b/qapi-schema.json
> @@ -2296,6 +2296,10 @@
>  #
>  # @queues: #optional number of queues to be created for multiqueue capable tap
>  #
> +# @forward_nic: #optional the name of host physical forward nic for COLO (Since 2.4)

Long line, please wrap to fit in 80 columns.

> +#
> +# @colo_script: #optional the script file which used by COLO (Since 2.4)

2.5 for both parameters.

New parameters should favor '-' over '_' except for consistency with
older commands; I guess the fact that we already have 'vnet_hdr'
qualifies as that unfortunate consistency.

Script files are very poor design.  Management apps like libvirt CANNOT
police what happens in the script file, and have to mark the entire
domain as untrusted if a script file is used.  Are you sure you can't
accomplish this through a saner method, such as allowing management to
pre-open an fd and pass that in to qemu, rather than having qemu execute
a script file?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]