| Message ID | 20190117185628.21862-12-crosa@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Acceptance Tests: target architecture support | expand |
On Thu, Jan 17, 2019 at 01:56:21PM -0500, Cleber Rosa wrote: > The set_console() utility function traditionally adds a device either > based on the explicitly given device type, or based on the machine type, > a known good type of device. > > But, for a number of machine types, it may be impossible or > inconvenient to add the devices my means of "-device" command line > options, and then it may better to just use the "-serial" option and > let QEMU itself, based on the machine type, set the device > accordingly. > > To achieve that, the behavior of set_console() now flags the intention > to add a console device on launch(), and if no explicit device type is > given, and there's no definition on CONSOLE_DEV_TYPES, the "-serial" > is going to be added to the QEMU command line, instead of raising > exceptions. > > Signed-off-by: Cleber Rosa <crosa@redhat.com> Reviewed-by: Caio Carrara <ccarrara@redhat.com> > --- > scripts/qemu.py | 28 +++++++++++++++------------- > 1 file changed, 15 insertions(+), 13 deletions(-) > {...} >
Hi Cleber, me again. :) On 01/17/2019 04:56 PM, Cleber Rosa wrote: > The set_console() utility function traditionally adds a device either > based on the explicitly given device type, or based on the machine type, > a known good type of device. > > But, for a number of machine types, it may be impossible or > inconvenient to add the devices my means of "-device" command line > options, and then it may better to just use the "-serial" option and > let QEMU itself, based on the machine type, set the device > accordingly. > > To achieve that, the behavior of set_console() now flags the intention > to add a console device on launch(), and if no explicit device type is > given, and there's no definition on CONSOLE_DEV_TYPES, the "-serial" > is going to be added to the QEMU command line, instead of raising > exceptions. > > Signed-off-by: Cleber Rosa <crosa@redhat.com> > --- > scripts/qemu.py | 28 +++++++++++++++------------- > 1 file changed, 15 insertions(+), 13 deletions(-) > > diff --git a/scripts/qemu.py b/scripts/qemu.py > index ec3567d4e2..88e1608b42 100644 > --- a/scripts/qemu.py > +++ b/scripts/qemu.py > @@ -121,6 +121,7 @@ class QEMUMachine(object): > self._temp_dir = None > self._launched = False > self._machine = None > + self._console_set = False > self._console_device_type = None > self._console_address = None > self._console_socket = None > @@ -240,13 +241,17 @@ class QEMUMachine(object): > '-display', 'none', '-vga', 'none'] > if self._machine is not None: > args.extend(['-machine', self._machine]) > - if self._console_device_type is not None: > + if self._console_set: > self._console_address = os.path.join(self._temp_dir, > self._name + "-console.sock") > chardev = ('socket,id=console,path=%s,server,nowait' % > self._console_address) > - device = '%s,chardev=console' % self._console_device_type > - args.extend(['-chardev', chardev, '-device', device]) > + args.extend(['-chardev', chardev]) > + if self._console_device_type is None: > + args.extend(['-serial', 'chardev:console']) > + else: > + device = '%s,chardev=console' % self._console_device_type > + args.extend(['-device', device]) > return args > > def _pre_launch(self): > @@ -479,23 +484,20 @@ class QEMUMachine(object): > machine launch time, as it depends on the temporary directory > to be created. > > - @param device_type: the device type, such as "isa-serial" > + @param device_type: the device type, such as "isa-serial". If > + None is given (the default value) a "-serial > + chardev:console" command line argument will > + be used instead, resorting to the machine's > + default device type. Shouldn't you mention it will look for device type on CONSOLE_DEV_TYPES if device_type is None and machine is not None? The description on set_console()'s docstring is out-of-sync with current implementation too. - Wainer > @raises: QEMUMachineAddDeviceError if the device type is not given > and can not be determined. > """ > - if device_type is None: > - if self._machine is None: > - raise QEMUMachineAddDeviceError("Can not add a console device:" > - " QEMU instance without a " > - "defined machine type") > + self._console_set = True > + if device_type is None and self._machine is not None: > for regex, device in CONSOLE_DEV_TYPES.items(): > if re.match(regex, self._machine): > device_type = device > break > - if device_type is None: > - raise QEMUMachineAddDeviceError("Can not add a console device:" > - " no matching console device " > - "type definition") > self._console_device_type = device_type > > @property
On 1/31/19 1:49 PM, Wainer dos Santos Moschetta wrote: > Hi Cleber, > > me again. :) > > On 01/17/2019 04:56 PM, Cleber Rosa wrote: >> The set_console() utility function traditionally adds a device either >> based on the explicitly given device type, or based on the machine type, >> a known good type of device. >> >> But, for a number of machine types, it may be impossible or >> inconvenient to add the devices my means of "-device" command line >> options, and then it may better to just use the "-serial" option and >> let QEMU itself, based on the machine type, set the device >> accordingly. >> >> To achieve that, the behavior of set_console() now flags the intention >> to add a console device on launch(), and if no explicit device type is >> given, and there's no definition on CONSOLE_DEV_TYPES, the "-serial" >> is going to be added to the QEMU command line, instead of raising >> exceptions. >> >> Signed-off-by: Cleber Rosa <crosa@redhat.com> >> --- >> scripts/qemu.py | 28 +++++++++++++++------------- >> 1 file changed, 15 insertions(+), 13 deletions(-) >> >> diff --git a/scripts/qemu.py b/scripts/qemu.py >> index ec3567d4e2..88e1608b42 100644 >> --- a/scripts/qemu.py >> +++ b/scripts/qemu.py >> @@ -121,6 +121,7 @@ class QEMUMachine(object): >> self._temp_dir = None >> self._launched = False >> self._machine = None >> + self._console_set = False >> self._console_device_type = None >> self._console_address = None >> self._console_socket = None >> @@ -240,13 +241,17 @@ class QEMUMachine(object): >> '-display', 'none', '-vga', 'none'] >> if self._machine is not None: >> args.extend(['-machine', self._machine]) >> - if self._console_device_type is not None: >> + if self._console_set: >> self._console_address = os.path.join(self._temp_dir, >> self._name + >> "-console.sock") >> chardev = ('socket,id=console,path=%s,server,nowait' % >> self._console_address) >> - device = '%s,chardev=console' % self._console_device_type >> - args.extend(['-chardev', chardev, '-device', device]) >> + args.extend(['-chardev', chardev]) >> + if self._console_device_type is None: >> + args.extend(['-serial', 'chardev:console']) >> + else: >> + device = '%s,chardev=console' % >> self._console_device_type >> + args.extend(['-device', device]) >> return args >> def _pre_launch(self): >> @@ -479,23 +484,20 @@ class QEMUMachine(object): >> machine launch time, as it depends on the temporary directory >> to be created. >> - @param device_type: the device type, such as "isa-serial" >> + @param device_type: the device type, such as "isa-serial". If >> + None is given (the default value) a "-serial >> + chardev:console" command line argument will >> + be used instead, resorting to the machine's >> + default device type. > > Shouldn't you mention it will look for device type on CONSOLE_DEV_TYPES > if device_type is None and machine is not None? > Yes, you're right. How about this: ... @param device_type: the device type, such as "isa-serial". If None is given (the default value) a "-serial chardev:console" command line argument will be used instead, resorting to the machine's default device type, if a machine type is set, and a matching entry exists on CONSOLE_DEV_TYPES. ... > The description on set_console()'s docstring is out-of-sync with current > implementation too. > Good catch! I'm rewriting that as: ... This is a convenience method that will either use the provided device type, of if not given, it will use the device type set on CONSOLE_DEV_TYPES if a machine type is set, and a matching entry exists on CONSOLE_DEV_TYPES. ... How does it sound? Thanks! - Cleber. > - Wainer > >> @raises: QEMUMachineAddDeviceError if the device type is not >> given >> and can not be determined. >> """ >> - if device_type is None: >> - if self._machine is None: >> - raise QEMUMachineAddDeviceError("Can not add a >> console device:" >> - " QEMU instance >> without a " >> - "defined machine type") >> + self._console_set = True >> + if device_type is None and self._machine is not None: >> for regex, device in CONSOLE_DEV_TYPES.items(): >> if re.match(regex, self._machine): >> device_type = device >> break >> - if device_type is None: >> - raise QEMUMachineAddDeviceError("Can not add a >> console device:" >> - " no matching console >> device " >> - "type definition") >> self._console_device_type = device_type >> @property > >
diff --git a/scripts/qemu.py b/scripts/qemu.py index ec3567d4e2..88e1608b42 100644 --- a/scripts/qemu.py +++ b/scripts/qemu.py @@ -121,6 +121,7 @@ class QEMUMachine(object): self._temp_dir = None self._launched = False self._machine = None + self._console_set = False self._console_device_type = None self._console_address = None self._console_socket = None @@ -240,13 +241,17 @@ class QEMUMachine(object): '-display', 'none', '-vga', 'none'] if self._machine is not None: args.extend(['-machine', self._machine]) - if self._console_device_type is not None: + if self._console_set: self._console_address = os.path.join(self._temp_dir, self._name + "-console.sock") chardev = ('socket,id=console,path=%s,server,nowait' % self._console_address) - device = '%s,chardev=console' % self._console_device_type - args.extend(['-chardev', chardev, '-device', device]) + args.extend(['-chardev', chardev]) + if self._console_device_type is None: + args.extend(['-serial', 'chardev:console']) + else: + device = '%s,chardev=console' % self._console_device_type + args.extend(['-device', device]) return args def _pre_launch(self): @@ -479,23 +484,20 @@ class QEMUMachine(object): machine launch time, as it depends on the temporary directory to be created. - @param device_type: the device type, such as "isa-serial" + @param device_type: the device type, such as "isa-serial". If + None is given (the default value) a "-serial + chardev:console" command line argument will + be used instead, resorting to the machine's + default device type. @raises: QEMUMachineAddDeviceError if the device type is not given and can not be determined. """ - if device_type is None: - if self._machine is None: - raise QEMUMachineAddDeviceError("Can not add a console device:" - " QEMU instance without a " - "defined machine type") + self._console_set = True + if device_type is None and self._machine is not None: for regex, device in CONSOLE_DEV_TYPES.items(): if re.match(regex, self._machine): device_type = device break - if device_type is None: - raise QEMUMachineAddDeviceError("Can not add a console device:" - " no matching console device " - "type definition") self._console_device_type = device_type @property
The set_console() utility function traditionally adds a device either based on the explicitly given device type, or based on the machine type, a known good type of device. But, for a number of machine types, it may be impossible or inconvenient to add the devices my means of "-device" command line options, and then it may better to just use the "-serial" option and let QEMU itself, based on the machine type, set the device accordingly. To achieve that, the behavior of set_console() now flags the intention to add a console device on launch(), and if no explicit device type is given, and there's no definition on CONSOLE_DEV_TYPES, the "-serial" is going to be added to the QEMU command line, instead of raising exceptions. Signed-off-by: Cleber Rosa <crosa@redhat.com> --- scripts/qemu.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-)