Re: [PATCH v5 09/12] runqemu: also accept -image suffix for rootfs parameter

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

On Mon, 2017-01-30 at 17:12 +0000, Bystricky, Juro wrote:
> 
> > -----Original Message-----
> > From: Patrick Ohly [mailto:patrick.ohly@intel.com]
> > Sent: Friday, January 27, 2017 11:22 AM
> > To: Bystricky, Juro <juro.bystricky@intel.com>
> > Cc: openembedded-core@lists.openembedded.org
> > Subject: Re: [OE-core] [PATCH v5 09/12] runqemu: also accept -image suffix
> > for rootfs parameter
> > 
> > On Fri, 2017-01-27 at 16:54 +0000, Bystricky, Juro wrote:
> > > Just curious: is this test for "image" in file name really necessary?
> > > With qemuboot.conf the relevant files are already spelled out.
> > > I don't see a need to force "compulsory" names for images.
> > > If I comment out this test, everything works just fine. Am I missing
> > something?
> > 
> > Some of the usages when checking for paths might have become obsolete,
> > but at least for distinguishing between machine and image base name
> > parameters it is still relevant:
> > 
> >     def check_args(self):
> > ...
> >             elif re.search(r'-image-|-image$', arg):
> >                 # Lazy rootfs
> >                 self.rootfs = arg
> >             elif arg.startswith('ovmf'):
> >                 self.ovmf_bios.append(arg)
> >             else:
> >                 # At last, assume is it the MACHINE
> >                 if (not unknown_arg) or unknown_arg == arg:
> >                     unknown_arg = arg
> >                 else:
> >                     raise Exception("Can't handle two unknown args: %s %
> > s" % (unknown_arg, arg))
> > 
> > When removing the "if re.search(r'-image-|-image$', arg)" clause one
> > gets an error for:
> > 
> > $ runqemu core-image-minimal ext4 qemux86
> > runqemu - ERROR - Can't handle two unknown args: core-image-minimal qemux86
> > runqemu - ERROR - Try 'runqemu help' on how to use it
> > 
> 
> I see, the purpose of this test is determine which argument is which,
> as they can be in any order. IMHO to differentiate between MACHINE and image it would 
> make more sense to search for "qemu" instead of "-image-" or "-image" .

The machine is not guaranteed to contain "qemu". I sent a patch to
meta-intel which enables "runqemu core-image-minimal ext4
intel-corei7-64", and other BSPs might want to do the same.

> (BTW do we need both -image- and -image$?)

Yes, for "core-image-minimal" and "foobar-installer-image" (something
that I am currently working on).

> There is also ANOTHER test for '-image-', in "is_deploy_dir_image". 
> This is the one I considered redundant (or not needed in case we have qemuboot.conf).

That might be true. I was trying to be conservative with this patch and
thus extended all existing checks instead of trying to to figure out
which one had become redundant.

-- 
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.