Re: [PATCH 4/7] runqemu: do not rely on grepping images

[Patrick Ohly <patrick.ohly@intel.com>]

On Thu, 2017-04-13 at 16:59 +0800, Robert Yang wrote:
> Hi Patrick,
> 
> On 04/13/2017 04:46 PM, Patrick Ohly wrote:
> > On Tue, 2017-04-11 at 02:21 -0700, Robert Yang wrote:
> >> Fixed when the image is large and not enough memory:
> >>   grep: memory exhausted
> >>   Aborted
> >>
> >> [YOCTO #11073]
> >>
> >> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> >> ---
> >>  meta/classes/qemuboot.bbclass |  3 +++
> >>  scripts/runqemu               | 19 +++++++++++--------
> >>  2 files changed, 14 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/meta/classes/qemuboot.bbclass b/meta/classes/qemuboot.bbclass
> >> index 3ca97cad4c..2870388dfb 100644
> >> --- a/meta/classes/qemuboot.bbclass
> >> +++ b/meta/classes/qemuboot.bbclass
> >> @@ -64,6 +64,9 @@ QB_DEFAULT_FSTYPE ?= "ext4"
> >>  QB_OPT_APPEND ?= "-show-cursor"
> >>  QB_NETWORK_DEVICE ?= "-device virtio-net-pci,netdev=net0,mac=@MAC@"
> >>
> >> +# This should be kept align with ROOT_VM
> >> +QB_DRIVE_TYPE ?= "/dev/sd"
> >
> > Now that I am faced with the problem of selecting virtio I'm starting to
> > wonder how to do that.
> >
> > Our refkit image doesn't expect the image on a particular drive, using
> > PARTUUID instead to find it. But when running under qemu, it would be
> > nice to use virtio for performance reasons... if the kernel supports it.
> > But that's not something that the image recipe or even the distro should
> > make assumptions about.
> >
> > It seems safer to let the machine configuration which chooses the kernel
> > also set QB_DRIVE_TYPE, but with ?= so that it can still be changed
> > elsewhere. In that case, we would need:
> >
> > qemuboot.bbclass: QB_DRIVE_TYPE ??= "/dev/sd"
> > qemuboot-intel.inc (from meta-intel): QB_DRIVE_TYPE ?= "/dev/vd"
> > distro, local or image (optional): QB_DRIVE_TYPE = "/dev/..."
> 
> The "??=" is not needed, just "?=" in qemuboot.bbclass is OK, we have
> "?=" in other bbclass, too, and they can be overrided as expected.

Other classes might depend on a different order of setting values.

> For example:
> 
> qemuboot-intel.inc (from meta-intel): QB_DRIVE_TYPE ?= "/dev/vd"

But you right, that works, because although that default gets set first,
it does not get overwritten by the latter default from qemuboot.bbclass.

I thought defaults worked the same way as normal assignments, i.e.
  foo ?= "a"
  foo ?= "b"
would result in foo == "b". That's not the case.

> >> +                elif self.get('QB_DRIVE_TYPE'):
> >> +                    drive_type = self.get('QB_DRIVE_TYPE')
> >> +                    if drive_type.startswith("/dev/sd"):
> >>                          logger.info('Using scsi drive')
> >>                          vm_drive = '-drive if=none,id=hd,file=%s,format=%s -device virtio-scsi-pci,id=scsi -device scsi-hd,drive=hd' \
> >>                                         % (self.rootfs, rootfs_format)
> >> -                    elif subprocess.call(cmd2, shell=True) == 0:
> >> +                    elif drive_type.startswith("/dev/hd"):
> >>                          logger.info('Using ide drive')
> >>                          vm_drive = "%s,format=%s" % (self.rootfs, rootfs_format)
> >>                      else:
> >> -                        logger.warn("Can't detect drive type %s" % self.rootfs)
> >> -                        logger.warn('Trying to use virtio block drive')
> >> -                        vm_drive = '-drive if=virtio,file=%s,format=%s' % (self.rootfs, rootfs_format)
> >> +                        logger.warn("Unknown QB_DRIVE_TYPE: %s" % drive_type)
> >> +
> >> +                if not vm_drive:
> >> +                    logger.warn("Failed to figure out drive type, consider define or fix QB_DRIVE_TYPE")
> >> +                    logger.warn('Trying to use virtio block drive')
> >> +                    vm_drive = '-drive if=virtio,file=%s,format=%s' % (self.rootfs, rootfs_format)
> >
> > Here "/dev/sd" should be treated as an explicit selection of virtio,
> > without triggering warnings.
> 
> Did you mean /dev/vd ? If yes, I'm fine with that.

Yes.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.