Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor QEMU tweaks for x86 and custom firmware #1484

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions doc/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2654,6 +2654,15 @@ Binds to:
dtb: 'dtb'
nic: 'user'

QEMUDriver:
qemu_bin: qemu-i386
machine: pc
cpu: core2duo
memory: 2G
extra_args: "-accel kvm"
nic: user,model=virtio-net-pci
disk: disk-image

.. code-block:: yaml

tools:
Expand All @@ -2670,10 +2679,10 @@ Implements:

Arguments:
- qemu_bin (str): reference to the tools key for the QEMU binary
- machine (str): QEMU machine type
- cpu (str): QEMU cpu type
- machine (str): optional, QEMU machine type
- cpu (str): optional, QEMU cpu type
- memory (str): QEMU memory size (ends with M or G)
- extra_args (str): extra QEMU arguments, they are passed directly to the QEMU binary
- extra_args (str): optional, extra QEMU arguments, they are passed directly to the QEMU binary
- boot_args (str): optional, additional kernel boot argument
- kernel (str): optional, reference to the images key for the kernel
- disk (str): optional, reference to the images key for the disk image
Expand Down
86 changes: 57 additions & 29 deletions labgrid/driver/qemudriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@ class QEMUDriver(ConsoleExpectMixin, Driver, PowerProtocol, ConsoleProtocol):

Args:
qemu_bin (str): reference to the tools key for the QEMU binary
machine (str): QEMU machine type
cpu (str): QEMU cpu type
memory (str): QEMU memory size (ends with M or G)
extra_args (str): extra QEMU arguments, they are passed directly to the QEMU binary
extra_args (str): optional, extra QEMU arguments, they are passed
directly to the QEMU binary
cpu (str): optional, QEMU cpu type
machine (str): optional, QEMU machine type
boot_args (str): optional, additional kernel boot argument
kernel (str): optional, reference to the images key for the kernel
disk (str): optional, reference to the images key for the disk image
Expand All @@ -51,10 +52,16 @@ class QEMUDriver(ConsoleExpectMixin, Driver, PowerProtocol, ConsoleProtocol):
nic (str): optional, configuration string to pass to QEMU to create a network interface
"""
qemu_bin = attr.ib(validator=attr.validators.instance_of(str))
machine = attr.ib(validator=attr.validators.instance_of(str))
cpu = attr.ib(validator=attr.validators.instance_of(str))
memory = attr.ib(validator=attr.validators.instance_of(str))
extra_args = attr.ib(validator=attr.validators.instance_of(str))
extra_args = attr.ib(
default=None,
validator=attr.validators.optional(attr.validators.instance_of(str)))
cpu = attr.ib(
default=None,
validator=attr.validators.optional(attr.validators.instance_of(str)))
machine = attr.ib(
default=None,
validator=attr.validators.optional(attr.validators.instance_of(str)))
boot_args = attr.ib(
default=None,
validator=attr.validators.optional(attr.validators.instance_of(str)))
Expand Down Expand Up @@ -98,6 +105,8 @@ def __attrs_post_init__(self):
self._tempdir = None
self._socket = None
self._clientsocket = None
self._bios_fname = None
self._sockpath = None
self._forwarded_ports = {}
atexit.register(self._atexit)

Expand All @@ -122,6 +131,17 @@ def get_qemu_version(self, qemu_bin):

return (int(m.group('major')), int(m.group('minor')), int(m.group('micro')))

def set_bios(self, bios):
"""Set the filename of the bios

This can be used by strategies to set the bios filename, overriding the
value provided in the environment.

Args:
bios (str): New bios filename
"""
self._bios_fname = bios

def get_qemu_base_args(self):
"""Returns the base command line used for Qemu without the options
related to QMP. These options can be used to start an interactive
Expand Down Expand Up @@ -156,7 +176,7 @@ def get_qemu_base_args(self):
cmd.append(
f"if=sd,format={disk_format},file={disk_path},id=mmc0{disk_opts}")
boot_args.append("root=/dev/mmcblk0p1 rootfstype=ext4 rootwait")
elif self.machine in ["pc", "q35", "virt"]:
elif self.machine in ["pc", "q35", "virt", None]:
cmd.append("-drive")
cmd.append(
f"if=virtio,format={disk_format},file={disk_path}{disk_opts}")
Expand Down Expand Up @@ -184,15 +204,20 @@ def get_qemu_base_args(self):
cmd.append("-bios")
cmd.append(
self.target.env.config.get_image_path(self.bios))

if "-append" in shlex.split(self.extra_args):
raise ExecutionError("-append in extra_args not allowed, use boot_args instead")

cmd.extend(shlex.split(self.extra_args))
cmd.append("-machine")
cmd.append(self.machine)
cmd.append("-cpu")
cmd.append(self.cpu)
elif self._bios_fname:
cmd.append("-bios")
cmd.append(self._bios_fname)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we can provide bios via the client arguments and than set _bios_fname via the set_bios() function. Why don't you modify bios instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well in this case 'bios' would not be provided.

The point here is that the strategy can provide the firmware, no matter what board type is chosen. We don't have to have a different 'bios' for every architecture / target.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case this driver is used by your strategy it is not provided, but it is still possible. Please modify this to use self.bios instead of the new hidden self._bios_fname.

if self.extra_args:
if "-append" in shlex.split(self.extra_args):
raise ExecutionError("-append in extra_args not allowed, use boot_args instead")

cmd.extend(shlex.split(self.extra_args))
if self.machine:
cmd.append("-machine")
cmd.append(self.machine)
if self.cpu:
cmd.append("-cpu")
cmd.append(self.cpu)
cmd.append("-m")
cmd.append(self.memory)
if self.display == "none":
Expand Down Expand Up @@ -226,22 +251,11 @@ def get_qemu_base_args(self):

def on_activate(self):
self._tempdir = tempfile.mkdtemp(prefix="labgrid-qemu-tmp-")
sockpath = f"{self._tempdir}/serialrw"
self._sockpath = f"{self._tempdir}/serialrw"
self._socket = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
self._socket.bind(sockpath)
self._socket.bind(self._sockpath)
self._socket.listen(0)

self._cmd = self.get_qemu_base_args()

self._cmd.append("-S")
self._cmd.append("-qmp")
self._cmd.append("stdio")

self._cmd.append("-chardev")
self._cmd.append(f"socket,id=serialsocket,path={sockpath}")
self._cmd.append("-serial")
self._cmd.append("chardev:serialsocket")

def on_deactivate(self):
if self.status:
self.off()
Expand All @@ -250,6 +264,7 @@ def on_deactivate(self):
self._clientsocket = None
self._socket.close()
self._socket = None
self._sockpath = None
shutil.rmtree(self._tempdir)

@step()
Expand All @@ -258,6 +273,17 @@ def on(self):
afterwards start the emulator using a QMP Command"""
if self.status:
return
self._cmd = self.get_qemu_base_args()

self._cmd.append("-S")
self._cmd.append("-qmp")
self._cmd.append("stdio")

self._cmd.append("-chardev")
self._cmd.append(f"socket,id=serialsocket,path={self._sockpath}")
self._cmd.append("-serial")
self._cmd.append("chardev:serialsocket")

self.logger.debug("Starting with: %s", self._cmd)
self._child = subprocess.Popen(
self._cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
Expand Down Expand Up @@ -328,6 +354,8 @@ def remove_port_forward(self, proto, local_address, local_port):
)

def _read(self, size=1, timeout=10, max_size=None):
if not self._clientsocket:
raise ExecutionError('QEMU has not been started')
ready, _, _ = select.select([self._clientsocket], [], [], timeout)
if ready:
# Collect some more data
Expand Down
Loading