* [KVM-AUTOTEST PATCH] KVM test: use command line option wrapper functions
2010-05-17 13:29 ` [KVM-AUTOTEST PATCH] KVM test: make use of tcpdump optional Michael Goldish
@ 2010-05-17 13:29 ` Michael Goldish
0 siblings, 0 replies; 5+ messages in thread
From: Michael Goldish @ 2010-05-17 13:29 UTC (permalink / raw)
To: autotest, kvm; +Cc: Michael Goldish
In order to support multiple versions of qemu which use different command line
options or syntaxes, wrap all command line options in small helper functions,
which append text to the command line according to the output of 'qemu -help'.
Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
client/tests/kvm/kvm_vm.py | 198 ++++++++++++++++++++++++++++++--------------
1 files changed, 135 insertions(+), 63 deletions(-)
diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 047505a..94bacdf 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -186,12 +186,100 @@ class VM:
nic_model -- string to pass as 'model' parameter for this
NIC (e.g. e1000)
"""
- if name is None:
- name = self.name
- if params is None:
- params = self.params
- if root_dir is None:
- root_dir = self.root_dir
+ # Helper function for command line option wrappers
+ def has_option(help, option):
+ return bool(re.search(r"^-%s(\s|$)" % option, help, re.MULTILINE))
+
+ # Wrappers for all supported qemu command line parameters.
+ # This is meant to allow support for multiple qemu versions.
+ # Each of these functions receives the output of 'qemu -help' as a
+ # parameter, and should add the requested command line option
+ # accordingly.
+
+ def add_name(help, name):
+ return " -name '%s'" % name
+
+ def add_unix_socket_monitor(help, filename):
+ return " -monitor unix:%s,server,nowait" % filename
+
+ def add_mem(help, mem):
+ return " -m %s" % mem
+
+ def add_smp(help, smp):
+ return " -smp %s" % smp
+
+ def add_cdrom(help, filename, index=2):
+ if has_option(help, "drive"):
+ return " -drive file=%s,index=%d,media=cdrom" % (filename,
+ index)
+ else:
+ return " -cdrom %s" % filename
+
+ def add_drive(help, filename, format=None, cache=None, werror=None,
+ serial=None, snapshot=False, boot=False):
+ cmd = " -drive file=%s" % filename
+ if format: cmd += ",if=%s" % format
+ if cache: cmd += ",cache=%s" % cache
+ if werror: cmd += ",werror=%s" % werror
+ if serial: cmd += ",serial=%s" % serial
+ if snapshot: cmd += ",snapshot=on"
+ if boot: cmd += ",boot=on"
+ return cmd
+
+ def add_nic(help, vlan, model=None, mac=None):
+ cmd = " -net nic,vlan=%d" % vlan
+ if model: cmd += ",model=%s" % model
+ if mac: cmd += ",macaddr=%s" % mac
+ return cmd
+
+ def add_net(help, vlan, mode, ifname=None, script=None,
+ downscript=None):
+ cmd = " -net %s,vlan=%d" % (mode, vlan)
+ if mode == "tap":
+ if ifname: cmd += ",ifname=%s" % ifname
+ if script: cmd += ",script=%s" % script
+ cmd += ",downscript=%s" % (downscript or "no")
+ return cmd
+
+ def add_floppy(help, filename):
+ return " -fda %s" % filename
+
+ def add_tftp(help, filename):
+ return " -tftp %s" % filename
+
+ def add_tcp_redir(help, host_port, guest_port):
+ return " -redir tcp:%s::%s" % (host_port, guest_port)
+
+ def add_vnc(help, vnc_port):
+ return " -vnc :%d" % (vnc_port - 5900)
+
+ def add_sdl(help):
+ if has_option(help, "sdl"):
+ return " -sdl"
+ else:
+ return ""
+
+ def add_nographic(help):
+ return " -nographic"
+
+ def add_uuid(help, uuid):
+ return " -uuid %s" % uuid
+
+ def add_pcidevice(help, host):
+ return " -pcidevice host=%s" % host
+
+ # End of command line option wrappers
+
+ if name is None: name = self.name
+ if params is None: params = self.params
+ if root_dir is None: root_dir = self.root_dir
+
+ qemu_binary = kvm_utils.get_path(root_dir, params.get("qemu_binary",
+ "qemu"))
+ # Get the output of 'qemu -help' (log a message in case this call never
+ # returns or causes some other kind of trouble)
+ logging.debug("Getting output of 'qemu -help'")
+ help = commands.getoutput("%s -help" % qemu_binary)
# Start constructing the qemu command
qemu_cmd = ""
@@ -199,65 +287,49 @@ class VM:
if params.get("x11_display"):
qemu_cmd += "DISPLAY=%s " % params.get("x11_display")
# Add the qemu binary
- qemu_cmd += kvm_utils.get_path(root_dir, params.get("qemu_binary",
- "qemu"))
+ qemu_cmd += qemu_binary
# Add the VM's name
- qemu_cmd += " -name '%s'" % name
+ qemu_cmd += add_name(help, name)
# Add the monitor socket parameter
- qemu_cmd += " -monitor unix:%s,server,nowait" % self.monitor_file_name
+ qemu_cmd += add_unix_socket_monitor(help, self.monitor_file_name)
for image_name in kvm_utils.get_sub_dict_names(params, "images"):
image_params = kvm_utils.get_sub_dict(params, image_name)
if image_params.get("boot_drive") == "no":
continue
- qemu_cmd += " -drive file=%s" % get_image_filename(image_params,
- root_dir)
- if image_params.get("drive_format"):
- qemu_cmd += ",if=%s" % image_params.get("drive_format")
- if image_params.get("drive_cache"):
- qemu_cmd += ",cache=%s" % image_params.get("drive_cache")
- if image_params.get("drive_werror"):
- qemu_cmd += ",werror=%s" % image_params.get("drive_werror")
- if image_params.get("drive_serial"):
- qemu_cmd += ",serial=%s" % image_params.get("drive_serial")
- if image_params.get("image_snapshot") == "yes":
- qemu_cmd += ",snapshot=on"
- if image_params.get("image_boot") == "yes":
- qemu_cmd += ",boot=on"
+ qemu_cmd += add_drive(help,
+ get_image_filename(image_params, root_dir),
+ image_params.get("drive_format"),
+ image_params.get("drive_cache"),
+ image_params.get("drive_werror"),
+ image_params.get("drive_serial"),
+ image_params.get("image_snapshot") == "yes",
+ image_params.get("image_boot") == "yes")
vlan = 0
for nic_name in kvm_utils.get_sub_dict_names(params, "nics"):
nic_params = kvm_utils.get_sub_dict(params, nic_name)
# Handle the '-net nic' part
- qemu_cmd += " -net nic,vlan=%d" % vlan
- if nic_params.get("nic_model"):
- qemu_cmd += ",model=%s" % nic_params.get("nic_model")
- if nic_params.has_key("address_index"):
- mac, ip = kvm_utils.get_mac_ip_pair_from_dict(nic_params)
- if mac:
- qemu_cmd += ",macaddr=%s" % mac
+ mac = None
+ if "address_index" in nic_params:
+ mac = kvm_utils.get_mac_ip_pair_from_dict(nic_params)[0]
+ qemu_cmd += add_nic(help, vlan, nic_params.get("nic_model"), mac)
# Handle the '-net tap' or '-net user' part
- mode = nic_params.get("nic_mode", "user")
- qemu_cmd += " -net %s,vlan=%d" % (mode, vlan)
- if mode == "tap":
- if nic_params.get("nic_ifname"):
- qemu_cmd += ",ifname=%s" % nic_params.get("nic_ifname")
- script_path = nic_params.get("nic_script")
- if script_path:
- script_path = kvm_utils.get_path(root_dir, script_path)
- qemu_cmd += ",script=%s" % script_path
- script_path = nic_params.get("nic_downscript")
- if script_path:
- script_path = kvm_utils.get_path(root_dir, script_path)
- qemu_cmd += ",downscript=%s" % script_path
- else:
- qemu_cmd += ",downscript=no"
+ script = nic_params.get("script")
+ downscript = nic_params.get("downscript")
+ if script:
+ script = kvm_utils.get_path(root_dir, script)
+ if downscript:
+ downscript = kvm_utils.get_path(root_dir, downscript)
+ qemu_cmd += add_net(help, vlan, nic_params.get("nic_mode", "user"),
+ nic_params.get("nic_ifname"),
+ script, downscript)
# Proceed to next NIC
vlan += 1
mem = params.get("mem")
if mem:
- qemu_cmd += " -m %s" % mem
+ qemu_cmd += add_mem(help, mem)
smp = params.get("smp")
if smp:
@@ -266,7 +338,7 @@ class VM:
iso = params.get("cdrom")
if iso:
iso = kvm_utils.get_path(root_dir, iso)
- qemu_cmd += " -drive file=%s,index=2,media=cdrom" % iso
+ qemu_cmd += add_cdrom(help, iso)
# Even though this is not a really scalable approach,
# it doesn't seem like we are going to need more than
@@ -274,47 +346,47 @@ class VM:
iso_extra = params.get("cdrom_extra")
if iso_extra:
iso_extra = kvm_utils.get_path(root_dir, iso_extra)
- qemu_cmd += " -drive file=%s,index=3,media=cdrom" % iso_extra
+ qemu_cmd += add_cdrom(help, iso_extra, 3)
# We may want to add {floppy_otps} parameter for -fda
- # {fat:floppy:}/path/. However vvfat is not usually recommended
+ # {fat:floppy:}/path/. However vvfat is not usually recommended.
floppy = params.get("floppy")
if floppy:
floppy = kvm_utils.get_path(root_dir, floppy)
- qemu_cmd += " -fda %s" % floppy
+ qemu_cmd += add_floppy(help, floppy)
tftp = params.get("tftp")
if tftp:
tftp = kvm_utils.get_path(root_dir, tftp)
- qemu_cmd += " -tftp %s" % tftp
-
- extra_params = params.get("extra_params")
- if extra_params:
- qemu_cmd += " %s" % extra_params
+ qemu_cmd += add_tftp(help, tftp)
for redir_name in kvm_utils.get_sub_dict_names(params, "redirs"):
redir_params = kvm_utils.get_sub_dict(params, redir_name)
guest_port = int(redir_params.get("guest_port"))
host_port = self.redirs.get(guest_port)
- qemu_cmd += " -redir tcp:%s::%s" % (host_port, guest_port)
+ qemu_cmd += add_tcp_redir(help, host_port, guest_port)
if params.get("display") == "vnc":
- qemu_cmd += " -vnc :%d" % (self.vnc_port - 5900)
+ qemu_cmd += add_vnc(help, self.vnc_port)
elif params.get("display") == "sdl":
- qemu_cmd += " -sdl"
+ qemu_cmd += add_sdl(help)
elif params.get("display") == "nographic":
- qemu_cmd += " -nographic"
+ qemu_cmd += add_nographic(help)
if params.get("uuid") == "random":
- qemu_cmd += " -uuid %s" % self.uuid
+ qemu_cmd += add_uuid(help, self.uuid)
elif params.get("uuid"):
- qemu_cmd += " -uuid %s" % params.get("uuid")
+ qemu_cmd += add_uuid(help, params.get("uuid"))
# If the PCI assignment step went OK, add each one of the PCI assigned
# devices to the qemu command line.
if self.pci_assignable:
for pci_id in self.pa_pci_ids:
- qemu_cmd += " -pcidevice host=%s" % pci_id
+ qemu_cmd += add_pcidevice(help, pci_id)
+
+ extra_params = params.get("extra_params")
+ if extra_params:
+ qemu_cmd += " %s" % extra_params
return qemu_cmd
--
1.5.4.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [KVM-AUTOTEST PATCH] KVM test: use command line option wrapper functions
[not found] <1112938281.779511274257387855.JavaMail.root@zmail04.collab.prod.int.phx2.redhat.com>
@ 2010-05-19 8:25 ` Feng Yang
2010-05-20 9:50 ` Michael Goldish
0 siblings, 1 reply; 5+ messages in thread
From: Feng Yang @ 2010-05-19 8:25 UTC (permalink / raw)
To: Michael Goldish; +Cc: autotest, kvm
Hi, Michael
Thanks for your patch.
We plan add "netdev" parameter support in make_qemu_command. Since you are working on this part. Could you add netdev support in your patch? hopeful netdev can be default supported in make_qemu_command if qemu support it. Thanks very much!
I think the point of this patch is good and we need this kinds of patch.
But I think we need not add so many new function. Especially some function only directly return the string and do nothing more.
This will increase the function call consumption.
----- "Michael Goldish" <mgoldish@redhat.com> wrote:
> From: "Michael Goldish" <mgoldish@redhat.com>
> To: autotest@test.kernel.org, kvm@vger.kernel.org
> Cc: "Michael Goldish" <mgoldish@redhat.com>
> Sent: Monday, May 17, 2010 9:29:35 PM GMT +08:00 Beijing / Chongqing / Hong Kong / Urumqi
> Subject: [KVM-AUTOTEST PATCH] KVM test: use command line option wrapper functions
>
> In order to support multiple versions of qemu which use different
> command line
> options or syntaxes, wrap all command line options in small helper
> functions,
> which append text to the command line according to the output of 'qemu
> -help'.
>
> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
> client/tests/kvm/kvm_vm.py | 198
> ++++++++++++++++++++++++++++++--------------
> 1 files changed, 135 insertions(+), 63 deletions(-)
>
> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> index 047505a..94bacdf 100755
> --- a/client/tests/kvm/kvm_vm.py
> +++ b/client/tests/kvm/kvm_vm.py
> @@ -186,12 +186,100 @@ class VM:
> nic_model -- string to pass as 'model' parameter for
> this
> NIC (e.g. e1000)
> """
> - if name is None:
> - name = self.name
> - if params is None:
> - params = self.params
> - if root_dir is None:
> - root_dir = self.root_dir
> + # Helper function for command line option wrappers
> + def has_option(help, option):
> + return bool(re.search(r"^-%s(\s|$)" % option, help,
> re.MULTILINE))
> +
> + # Wrappers for all supported qemu command line parameters.
> + # This is meant to allow support for multiple qemu versions.
> + # Each of these functions receives the output of 'qemu -help'
> as a
> + # parameter, and should add the requested command line
> option
> + # accordingly.
> +
> + def add_name(help, name):
> + return " -name '%s'" % name
I think we need not add so many new function. Especially some function only directly return the string and do nothing more.
This will increase the function call consumption.
> +
> + def add_unix_socket_monitor(help, filename):
> + return " -monitor unix:%s,server,nowait" % filename
Same as above
> +
> + def add_mem(help, mem):
> + return " -m %s" % mem
Same as above
> +
> + def add_smp(help, smp):
> + return " -smp %s" % smp
Same as above.
> +
> + def add_cdrom(help, filename, index=2):
> + if has_option(help, "drive"):
> + return " -drive file=%s,index=%d,media=cdrom" %
> (filename,
> +
> index)
> + else:
> + return " -cdrom %s" % filename
> +
> + def add_drive(help, filename, format=None, cache=None,
> werror=None,
> + serial=None, snapshot=False, boot=False):
> + cmd = " -drive file=%s" % filename
> + if format: cmd += ",if=%s" % format
> + if cache: cmd += ",cache=%s" % cache
> + if werror: cmd += ",werror=%s" % werror
> + if serial: cmd += ",serial=%s" % serial
> + if snapshot: cmd += ",snapshot=on"
> + if boot: cmd += ",boot=on"
> + return cmd
> +
> + def add_nic(help, vlan, model=None, mac=None):
> + cmd = " -net nic,vlan=%d" % vlan
> + if model: cmd += ",model=%s" % model
> + if mac: cmd += ",macaddr=%s" % mac
> + return cmd
> +
> + def add_net(help, vlan, mode, ifname=None, script=None,
> + downscript=None):
> + cmd = " -net %s,vlan=%d" % (mode, vlan)
> + if mode == "tap":
> + if ifname: cmd += ",ifname=%s" % ifname
> + if script: cmd += ",script=%s" % script
> + cmd += ",downscript=%s" % (downscript or "no")
> + return cmd
> +
> + def add_floppy(help, filename):
> + return " -fda %s" % filename
> +
> + def add_tftp(help, filename):
> + return " -tftp %s" % filename
> +
> + def add_tcp_redir(help, host_port, guest_port):
> + return " -redir tcp:%s::%s" % (host_port, guest_port)
> +
> + def add_vnc(help, vnc_port):
> + return " -vnc :%d" % (vnc_port - 5900)
> +
> + def add_sdl(help):
> + if has_option(help, "sdl"):
> + return " -sdl"
> + else:
> + return ""
> +
> + def add_nographic(help):
> + return " -nographic"
> +
> + def add_uuid(help, uuid):
> + return " -uuid %s" % uuid
> +
> + def add_pcidevice(help, host):
> + return " -pcidevice host=%s" % host
> +
> + # End of command line option wrappers
> +
> + if name is None: name = self.name
> + if params is None: params = self.params
> + if root_dir is None: root_dir = self.root_dir
> +
> + qemu_binary = kvm_utils.get_path(root_dir,
> params.get("qemu_binary",
> +
> "qemu"))
> + # Get the output of 'qemu -help' (log a message in case this
> call never
> + # returns or causes some other kind of trouble)
> + logging.debug("Getting output of 'qemu -help'")
> + help = commands.getoutput("%s -help" % qemu_binary)
>
> # Start constructing the qemu command
> qemu_cmd = ""
> @@ -199,65 +287,49 @@ class VM:
> if params.get("x11_display"):
> qemu_cmd += "DISPLAY=%s " % params.get("x11_display")
> # Add the qemu binary
> - qemu_cmd += kvm_utils.get_path(root_dir,
> params.get("qemu_binary",
> - "qemu"))
> + qemu_cmd += qemu_binary
> # Add the VM's name
> - qemu_cmd += " -name '%s'" % name
> + qemu_cmd += add_name(help, name)
> # Add the monitor socket parameter
> - qemu_cmd += " -monitor unix:%s,server,nowait" %
> self.monitor_file_name
> + qemu_cmd += add_unix_socket_monitor(help,
> self.monitor_file_name)
>
> for image_name in kvm_utils.get_sub_dict_names(params,
> "images"):
> image_params = kvm_utils.get_sub_dict(params,
> image_name)
> if image_params.get("boot_drive") == "no":
> continue
> - qemu_cmd += " -drive file=%s" %
> get_image_filename(image_params,
> -
> root_dir)
> - if image_params.get("drive_format"):
> - qemu_cmd += ",if=%s" %
> image_params.get("drive_format")
> - if image_params.get("drive_cache"):
> - qemu_cmd += ",cache=%s" %
> image_params.get("drive_cache")
> - if image_params.get("drive_werror"):
> - qemu_cmd += ",werror=%s" %
> image_params.get("drive_werror")
> - if image_params.get("drive_serial"):
> - qemu_cmd += ",serial=%s" %
> image_params.get("drive_serial")
> - if image_params.get("image_snapshot") == "yes":
> - qemu_cmd += ",snapshot=on"
> - if image_params.get("image_boot") == "yes":
> - qemu_cmd += ",boot=on"
> + qemu_cmd += add_drive(help,
> + get_image_filename(image_params,
> root_dir),
> + image_params.get("drive_format"),
> + image_params.get("drive_cache"),
> + image_params.get("drive_werror"),
> + image_params.get("drive_serial"),
> + image_params.get("image_snapshot")
> == "yes",
> + image_params.get("image_boot") ==
> "yes")
>
> vlan = 0
> for nic_name in kvm_utils.get_sub_dict_names(params,
> "nics"):
> nic_params = kvm_utils.get_sub_dict(params, nic_name)
> # Handle the '-net nic' part
> - qemu_cmd += " -net nic,vlan=%d" % vlan
> - if nic_params.get("nic_model"):
> - qemu_cmd += ",model=%s" %
> nic_params.get("nic_model")
> - if nic_params.has_key("address_index"):
> - mac, ip =
> kvm_utils.get_mac_ip_pair_from_dict(nic_params)
> - if mac:
> - qemu_cmd += ",macaddr=%s" % mac
> + mac = None
> + if "address_index" in nic_params:
> + mac =
> kvm_utils.get_mac_ip_pair_from_dict(nic_params)[0]
> + qemu_cmd += add_nic(help, vlan,
> nic_params.get("nic_model"), mac)
> # Handle the '-net tap' or '-net user' part
> - mode = nic_params.get("nic_mode", "user")
> - qemu_cmd += " -net %s,vlan=%d" % (mode, vlan)
> - if mode == "tap":
> - if nic_params.get("nic_ifname"):
> - qemu_cmd += ",ifname=%s" %
> nic_params.get("nic_ifname")
> - script_path = nic_params.get("nic_script")
> - if script_path:
> - script_path = kvm_utils.get_path(root_dir,
> script_path)
> - qemu_cmd += ",script=%s" % script_path
> - script_path = nic_params.get("nic_downscript")
> - if script_path:
> - script_path = kvm_utils.get_path(root_dir,
> script_path)
> - qemu_cmd += ",downscript=%s" % script_path
> - else:
> - qemu_cmd += ",downscript=no"
> + script = nic_params.get("script")
> + downscript = nic_params.get("downscript")
> + if script:
> + script = kvm_utils.get_path(root_dir, script)
> + if downscript:
> + downscript = kvm_utils.get_path(root_dir,
> downscript)
> + qemu_cmd += add_net(help, vlan,
> nic_params.get("nic_mode", "user"),
> + nic_params.get("nic_ifname"),
> + script, downscript)
> # Proceed to next NIC
> vlan += 1
>
> mem = params.get("mem")
> if mem:
> - qemu_cmd += " -m %s" % mem
> + qemu_cmd += add_mem(help, mem)
>
> smp = params.get("smp")
> if smp:
> @@ -266,7 +338,7 @@ class VM:
> iso = params.get("cdrom")
> if iso:
> iso = kvm_utils.get_path(root_dir, iso)
> - qemu_cmd += " -drive file=%s,index=2,media=cdrom" % iso
> + qemu_cmd += add_cdrom(help, iso)
>
> # Even though this is not a really scalable approach,
> # it doesn't seem like we are going to need more than
> @@ -274,47 +346,47 @@ class VM:
> iso_extra = params.get("cdrom_extra")
> if iso_extra:
> iso_extra = kvm_utils.get_path(root_dir, iso_extra)
> - qemu_cmd += " -drive file=%s,index=3,media=cdrom" %
> iso_extra
> + qemu_cmd += add_cdrom(help, iso_extra, 3)
>
> # We may want to add {floppy_otps} parameter for -fda
> - # {fat:floppy:}/path/. However vvfat is not usually
> recommended
> + # {fat:floppy:}/path/. However vvfat is not usually
> recommended.
> floppy = params.get("floppy")
> if floppy:
> floppy = kvm_utils.get_path(root_dir, floppy)
> - qemu_cmd += " -fda %s" % floppy
> + qemu_cmd += add_floppy(help, floppy)
>
> tftp = params.get("tftp")
> if tftp:
> tftp = kvm_utils.get_path(root_dir, tftp)
> - qemu_cmd += " -tftp %s" % tftp
> -
> - extra_params = params.get("extra_params")
> - if extra_params:
> - qemu_cmd += " %s" % extra_params
> + qemu_cmd += add_tftp(help, tftp)
>
> for redir_name in kvm_utils.get_sub_dict_names(params,
> "redirs"):
> redir_params = kvm_utils.get_sub_dict(params,
> redir_name)
> guest_port = int(redir_params.get("guest_port"))
> host_port = self.redirs.get(guest_port)
> - qemu_cmd += " -redir tcp:%s::%s" % (host_port,
> guest_port)
> + qemu_cmd += add_tcp_redir(help, host_port, guest_port)
>
> if params.get("display") == "vnc":
> - qemu_cmd += " -vnc :%d" % (self.vnc_port - 5900)
> + qemu_cmd += add_vnc(help, self.vnc_port)
> elif params.get("display") == "sdl":
> - qemu_cmd += " -sdl"
> + qemu_cmd += add_sdl(help)
> elif params.get("display") == "nographic":
> - qemu_cmd += " -nographic"
> + qemu_cmd += add_nographic(help)
>
> if params.get("uuid") == "random":
> - qemu_cmd += " -uuid %s" % self.uuid
> + qemu_cmd += add_uuid(help, self.uuid)
> elif params.get("uuid"):
> - qemu_cmd += " -uuid %s" % params.get("uuid")
> + qemu_cmd += add_uuid(help, params.get("uuid"))
>
> # If the PCI assignment step went OK, add each one of the PCI
> assigned
> # devices to the qemu command line.
> if self.pci_assignable:
> for pci_id in self.pa_pci_ids:
> - qemu_cmd += " -pcidevice host=%s" % pci_id
> + qemu_cmd += add_pcidevice(help, pci_id)
> +
> + extra_params = params.get("extra_params")
> + if extra_params:
> + qemu_cmd += " %s" % extra_params
>
> return qemu_cmd
>
> --
> 1.5.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [KVM-AUTOTEST PATCH] KVM test: use command line option wrapper functions
2010-05-19 8:25 ` [KVM-AUTOTEST PATCH] KVM test: use command line option wrapper functions Feng Yang
@ 2010-05-20 9:50 ` Michael Goldish
2010-05-20 10:57 ` Lucas Meneghel Rodrigues
0 siblings, 1 reply; 5+ messages in thread
From: Michael Goldish @ 2010-05-20 9:50 UTC (permalink / raw)
To: Feng Yang; +Cc: autotest, kvm
On 05/19/2010 11:25 AM, Feng Yang wrote:
> Hi, Michael
>
> Thanks for your patch.
> We plan add "netdev" parameter support in make_qemu_command. Since you are working on this part. Could you add netdev support in your patch? hopeful netdev can be default supported in make_qemu_command if qemu support it. Thanks very much!
Sure, I'll look into it.
> I think the point of this patch is good and we need this kinds of patch.
> But I think we need not add so many new function. Especially some function only directly return the string and do nothing more.
> This will increase the function call consumption.
>
>
> ----- "Michael Goldish" <mgoldish@redhat.com> wrote:
>
>> From: "Michael Goldish" <mgoldish@redhat.com>
>> To: autotest@test.kernel.org, kvm@vger.kernel.org
>> Cc: "Michael Goldish" <mgoldish@redhat.com>
>> Sent: Monday, May 17, 2010 9:29:35 PM GMT +08:00 Beijing / Chongqing / Hong Kong / Urumqi
>> Subject: [KVM-AUTOTEST PATCH] KVM test: use command line option wrapper functions
>>
>> In order to support multiple versions of qemu which use different
>> command line
>> options or syntaxes, wrap all command line options in small helper
>> functions,
>> which append text to the command line according to the output of 'qemu
>> -help'.
>>
>> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
>> ---
>> client/tests/kvm/kvm_vm.py | 198
>> ++++++++++++++++++++++++++++++--------------
>> 1 files changed, 135 insertions(+), 63 deletions(-)
>>
>> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
>> index 047505a..94bacdf 100755
>> --- a/client/tests/kvm/kvm_vm.py
>> +++ b/client/tests/kvm/kvm_vm.py
>> @@ -186,12 +186,100 @@ class VM:
>> nic_model -- string to pass as 'model' parameter for
>> this
>> NIC (e.g. e1000)
>> """
>> - if name is None:
>> - name = self.name
>> - if params is None:
>> - params = self.params
>> - if root_dir is None:
>> - root_dir = self.root_dir
>> + # Helper function for command line option wrappers
>> + def has_option(help, option):
>> + return bool(re.search(r"^-%s(\s|$)" % option, help,
>> re.MULTILINE))
>> +
>> + # Wrappers for all supported qemu command line parameters.
>> + # This is meant to allow support for multiple qemu versions.
>> + # Each of these functions receives the output of 'qemu -help'
>> as a
>> + # parameter, and should add the requested command line
>> option
>> + # accordingly.
>> +
>> + def add_name(help, name):
>> + return " -name '%s'" % name
>
> I think we need not add so many new function. Especially some function only directly return the string and do nothing more.
> This will increase the function call consumption.
>
>> +
>> + def add_unix_socket_monitor(help, filename):
>> + return " -monitor unix:%s,server,nowait" % filename
> Same as above
>> +
>> + def add_mem(help, mem):
>> + return " -m %s" % mem
> Same as above
>> +
>> + def add_smp(help, smp):
>> + return " -smp %s" % smp
> Same as above.
All these helper functions are meant to be extended and modified in the
future. They're only there to minimize future effort involved in adding
support for new command line syntaxes.
Right now add_smp() just returns " -smp %s", but in the future we may
have to support different syntaxes for -smp, and then add_smp() will
consult the output of 'qemu -help' and return the proper string.
What do you mean by function call consumption? I don't think these
functions cause a measurable slowdown, and make_qemu_command() is called
very few times, so this really isn't a concern IMO.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [KVM-AUTOTEST PATCH] KVM test: use command line option wrapper functions
2010-05-20 9:50 ` Michael Goldish
@ 2010-05-20 10:57 ` Lucas Meneghel Rodrigues
0 siblings, 0 replies; 5+ messages in thread
From: Lucas Meneghel Rodrigues @ 2010-05-20 10:57 UTC (permalink / raw)
To: Michael Goldish; +Cc: Feng Yang, autotest, kvm
On Thu, 2010-05-20 at 12:50 +0300, Michael Goldish wrote:
> On 05/19/2010 11:25 AM, Feng Yang wrote:
> > Hi, Michael
> >
> > Thanks for your patch.
> > We plan add "netdev" parameter support in make_qemu_command. Since you are working on this part. Could you add netdev support in your patch? hopeful netdev can be default supported in make_qemu_command if qemu support it. Thanks very much!
>
> Sure, I'll look into it.
>
> > I think the point of this patch is good and we need this kinds of patch.
> > But I think we need not add so many new function. Especially some function only directly return the string and do nothing more.
> > This will increase the function call consumption.
> >
> All these helper functions are meant to be extended and modified in the
> future. They're only there to minimize future effort involved in adding
> support for new command line syntaxes.
> Right now add_smp() just returns " -smp %s", but in the future we may
> have to support different syntaxes for -smp, and then add_smp() will
> consult the output of 'qemu -help' and return the proper string.
> What do you mean by function call consumption? I don't think these
> functions cause a measurable slowdown, and make_qemu_command() is called
> very few times, so this really isn't a concern IMO.
Agreed, the wrappers are a good strategy in the case we have to support
different feature sets and syntax.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [KVM-AUTOTEST PATCH] KVM test: use command line option wrapper functions
[not found] <1160970060.953181274407267225.JavaMail.root@zmail04.collab.prod.int.phx2.redhat.com>
@ 2010-05-21 2:03 ` Feng Yang
0 siblings, 0 replies; 5+ messages in thread
From: Feng Yang @ 2010-05-21 2:03 UTC (permalink / raw)
To: Michael Goldish; +Cc: autotest, kvm, lmr
----- "Lucas Meneghel Rodrigues" <lmr@redhat.com> wrote:
> From: "Lucas Meneghel Rodrigues" <lmr@redhat.com>
> To: "Michael Goldish" <mgoldish@redhat.com>
> Cc: "Feng Yang" <fyang@redhat.com>, autotest@test.kernel.org, kvm@vger.kernel.org
> Sent: Thursday, May 20, 2010 6:57:23 PM GMT +08:00 Beijing / Chongqing / Hong Kong / Urumqi
> Subject: Re: [KVM-AUTOTEST PATCH] KVM test: use command line option wrapper functions
>
> On Thu, 2010-05-20 at 12:50 +0300, Michael Goldish wrote:
> > On 05/19/2010 11:25 AM, Feng Yang wrote:
> > > Hi, Michael
> > >
> > > Thanks for your patch.
> > > We plan add "netdev" parameter support in make_qemu_command.
> Since you are working on this part. Could you add netdev support in
> your patch? hopeful netdev can be default supported in
> make_qemu_command if qemu support it. Thanks very much!
> >
> > Sure, I'll look into it.
> >
> > > I think the point of this patch is good and we need this kinds of
> patch.
> > > But I think we need not add so many new function. Especially some
> function only directly return the string and do nothing more.
> > > This will increase the function call consumption.
> > >
> > All these helper functions are meant to be extended and modified in
> the
> > future. They're only there to minimize future effort involved in
> adding
> > support for new command line syntaxes.
> > Right now add_smp() just returns " -smp %s", but in the future we
> may
> > have to support different syntaxes for -smp, and then add_smp()
> will
> > consult the output of 'qemu -help' and return the proper string.
> > What do you mean by function call consumption? I don't think these
> > functions cause a measurable slowdown, and make_qemu_command() is
> called
> > very few times, so this really isn't a concern IMO.
>
> Agreed, the wrappers are a good strategy in the case we have to
> support
> different feature sets and syntax.
I know your meaning. Yes, the wrapper is a good strategy for some parameters.
For the new added feature which could not work on old kvm, We need check support
before using it. So i say we need this patch. But i still think so many one line function
is not a good code style.
For many parameters that already support by all the kvm build, I do not think
it will have big change on syntax later.
Because a mature software should ensure the stability of the interface.
So we need not add one line function for these parameters now.
Even these parameters change its interface later, then we update the code is ok.
Again. Thanks for your patch and comments.
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-05-21 2:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1112938281.779511274257387855.JavaMail.root@zmail04.collab.prod.int.phx2.redhat.com>
2010-05-19 8:25 ` [KVM-AUTOTEST PATCH] KVM test: use command line option wrapper functions Feng Yang
2010-05-20 9:50 ` Michael Goldish
2010-05-20 10:57 ` Lucas Meneghel Rodrigues
[not found] <1160970060.953181274407267225.JavaMail.root@zmail04.collab.prod.int.phx2.redhat.com>
2010-05-21 2:03 ` Feng Yang
2010-05-17 13:29 [KVM-AUTOTEST PATCH] KVM test: formatting improvements to scan_results.py Michael Goldish
2010-05-17 13:29 ` [KVM-AUTOTEST PATCH] KVM test: make use of tcpdump optional Michael Goldish
2010-05-17 13:29 ` [KVM-AUTOTEST PATCH] KVM test: use command line option wrapper functions Michael Goldish
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).