Re: [dts] [PATCH] framework/qemu_kvm: Add machine and pflash option support

["Liu, Yong" <yong.liu@intel.com>]

Hi Phil,
Overall I'm fine with your patch. Just two comments as below.

1. I can't find code for the fix which mentioned in commit log line 3.
2. Since you are adding some options for virtual machine, please add some descriptions in doc/dts_gsg/virtualization.rst and reference configuration file.

Thanks,
Marvin

> -----Original Message-----
> From: dts [mailto:dts-bounces@dpdk.org] On Behalf Of Phil Yang
> Sent: Wednesday, November 22, 2017 5:37 PM
> To: dts@dpdk.org
> Cc: nd@arm.com; Jianbo.Liu@arm.com; Herbert.Guan@arm.com;
> phil.yang@arm.com
> Subject: [dts] [PATCH] framework/qemu_kvm: Add machine and pflash option
> support
> 
> 1. Add machine and pflash option support;
> 2. Set default machine type 'virt' for Aarch64;
> 3. Fix Aarch64 vhost vm case failure when using the default "-net nic
> model".
>    since the default nic type in vm is "virtio".
> 4. Add opt_format, opt_if, opt_index, opt_media for disk option.
> 
> Signed-off-by: Phil Yang <phil.yang@arm.com>
> ---
>  conf/vhost_sample.cfg | 10 ++++++++++
>  framework/qemu_kvm.py | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+)
> 
> diff --git a/conf/vhost_sample.cfg b/conf/vhost_sample.cfg
> index 8bdc20e..ab71936 100644
> --- a/conf/vhost_sample.cfg
> +++ b/conf/vhost_sample.cfg
> @@ -2,6 +2,9 @@
>  # name
>  #       name: vm0
>  #
> +# machine
> +#       machine: [virt | ...]
> +#
>  # enable_kvm
>  #       enable: [yes | no]
>  #
> @@ -18,6 +21,13 @@
>  #
>  # disk
>  #       file: /path/to/image/test.img
> +#       opt_format: raw
> +#       opt_if: virtio
> +#       opt_index: 0
> +#       opt_media: disk
> +#
> +# pflash
> +#       file: /path/to/image/pflash0.img
>  #
>  # char
>  #       opt_path: define the file path to vhost-net socket file
> diff --git a/framework/qemu_kvm.py b/framework/qemu_kvm.py
> index 79095bd..42acd69 100644
> --- a/framework/qemu_kvm.py
> +++ b/framework/qemu_kvm.py
> @@ -98,12 +98,17 @@ class QEMUKvm(VirtBase):
>          # internal variable to track whether default nic has been added
>          self.__default_nic = False
> 
> +        # arch info for multi-paltform init
> +        self.arch = self.host_session.send_expect('uname -m', '# ')
> +
>          # set some default values for vm,
>          # if there is not the values of the specified options
>          self.set_vm_default()
> 
>      def set_vm_default(self):
>          self.set_vm_name(self.vm_name)
> +        if self.arch == 'aarch64':
> +            self.set_vm_machine('virt')
>          self.set_vm_enable_kvm()
>          self.set_vm_pid_file()
>          self.set_vm_qga()
> @@ -249,6 +254,25 @@ class QEMUKvm(VirtBase):
>              enable_kvm_boot_line = '-enable-kvm'
>              self.__add_boot_line(enable_kvm_boot_line)
> 
> +    def set_vm_machine(self, machine):
> +        """
> +        Set VM boot option to specify the option 'machine'.
> +        """
> +        index = self.find_option_index('machine')
> +        if index:
> +            self.params[index] = {'machine': [{'machine': '%s' %
> machine}]}
> +        else:
> +            self.params.append({'machine': [{'machine': '%s' % machine}]})
> +
> +    def add_vm_machine(self, **options):
> +        """
> +        'machine': 'virt'
> +        """
> +        if 'machine' in options.keys() and \
> +                options['machine']:
> +            machine_boot_line = '-machine %s' % options['machine']
> +            self.__add_boot_line(machine_boot_line)
> +
>      def set_vm_pid_file(self):
>          """
>          Set VM pidfile option for manage qemu process
> @@ -329,11 +353,32 @@ class QEMUKvm(VirtBase):
>      def add_vm_disk(self, **options):
>          """
>          file: /home/image/test.img
> +        opt_format: raw
> +        opt_if: virtio
> +        opt_index: 0
> +        opt_media: disk
>          """
> +        separator = ','
>          if 'file' in options.keys():
>              disk_boot_line = '-drive file=%s' % options['file']
> +            if 'opt_format' in options.keys():
> +                disk_boot_line += separator + 'format=%s' %
> options['opt_format']
> +            if 'opt_if' in options.keys():
> +                disk_boot_line += separator + 'if=%s' % options['opt_if']
> +            if 'opt_index' in options.keys():
> +                disk_boot_line += separator + 'index=%s' %
> options['opt_index']
> +            if 'opt_media' in options.keys():
> +                disk_boot_line += separator + 'media=%s' %
> options['opt_media']
>              self.__add_boot_line(disk_boot_line)
> 
> +    def add_vm_pflash(self, **options):
> +        """
> +        file: /home/image/flash0.img
> +        """
> +        if 'file' in options.keys():
> +            pflash_boot_line = '-pflash %s' % options['file']
> +            self.__add_boot_line(pflash_boot_line)
> +
>      def add_vm_login(self, **options):
>          """
>          user: login username of virtual machine
> --
> 2.7.4