On Mon, 2017-01-09 at 19:50 -0800, Ricardo Neri wrote: > On Wed, 2017-01-04 at 10:43 +0100, Patrick Ohly wrote: > > To > > test that supporting separate code and variables also works, I've > > implemented that locally so that ovmf.fd ovmf_secboot.fd, ovmf_code.fd, > > ovmf_secboot_code.fd and ovmf_vars.fd get deployed and runqemu supports > > more than one "ovmf" parameter - this worked nicely. Full change below. > > ... Now that you have took the time to prototype the solution, we could > put it to use. > > > > > Now that I've implemented it, I wonder whether it would be worth > > submitting that as part of rev2 of this patch series. Any opinions? > > > > diff --git a/meta/recipes-core/ovmf/ovmf_git.bb > > b/meta/recipes-core/ovmf/ovmf_git.bb > > index ef61b16..391274b 100644 > > --- a/meta/recipes-core/ovmf/ovmf_git.bb > > +++ b/meta/recipes-core/ovmf/ovmf_git.bb > > @@ -125,6 +125,8 @@ do_compile_class-target() { > > rm -rf ${S}/Build/Ovmf$OVMF_DIR_SUFFIX > > ${S}/OvmfPkg/build.sh $PARALLEL_JOBS -a $OVMF_ARCH -b RELEASE -t > > ${FIXED_GCCVER} > > ln ${build_dir}/FV/OVMF.fd ${WORKDIR}/ovmf/OVMF.fd > > + ln ${build_dir}/FV/OVMF_CODE.fd ${WORKDIR}/ovmf/OVMF.code.fd > > + ln ${build_dir}/FV/OVMF_VARS.fd ${WORKDIR}/ovmf/OVMF.vars.fd > > > > # See CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt and > > # https://src.fedoraproject.org/cgit/rpms/edk2.git/tree/ for > > @@ -137,6 +139,7 @@ do_compile_class-target() { > > ( cd ${S}/CryptoPkg/Library/OpensslLib/ && ./Install.sh ) > > ${S}/OvmfPkg/build.sh $PARALLEL_JOBS -a $OVMF_ARCH -b RELEASE -t > > ${FIXED_GCCVER} ${OVMF_SECURE_BOOT_FLAGS} > > ln ${build_dir}/FV/OVMF.fd ${WORKDIR}/ovmf/OVMF.secboot.fd > > + ln ${build_dir}/FV/OVMF_CODE.fd ${WORKDIR}/ovmf/OVMF.code.secboot.fd > > for i in Shell.efi EnrollDefaultKeys.efi; do > > ln ${build_dir}/${OVMF_ARCH}/$i ${WORKDIR}/ovmf/$i > > done > > @@ -170,8 +173,9 @@ do_deploy() { > > } > > do_deploy_class-target() { > > # For use with "runqemu ovmf". > > - qemu-img convert -f raw -O qcow2 ${WORKDIR}/ovmf/OVMF.fd > > ${DEPLOYDIR}/ovmf.qcow2 > > - qemu-img convert -f raw -O qcow2 ${WORKDIR}/ovmf/OVMF.secboot.fd > > ${DEPLOYDIR}/ovmf.secboot.qcow2 > > + for i in OVMF OVMF.secboot OVMF.code OVMF.vars OVMF.code.secboot; do > > + qemu-img convert -f raw -O qcow2 ${WORKDIR}/ovmf/$i.fd > > ${DEPLOYDIR}/`echo $i | tr A-Z a-z`.qcow2 > > Will this preserve any previous OVMF_vars.fd that might exist in the > directory.
No, it will overwrite ovmf.vars.qcow2 and any variables stored in that get lost. That is consistent with rebuilding the disk image: a user who wants to have a "persistent" virtual machine must copy the relevant file and then use the full file paths. In this case, that means invoking runqemu with the file path to the copy of ovmf.vars.qcow2 instead of just "ovmf.vars". > > if self.ovmf_bios: > > - print('OVMF: [%s]' % self.ovmf_bios) > > + print('OVMF: %s' % self.ovmf_bios) > > Is there a reason to remove the brackets here? self.ovmf_bios is a list, so formatting it as string will add the brackets. I found that more readable than the (from a semantic point of view more correct) double brackets: [['.../ovmf.code.qcow2', '.../ovmf.vars.qcow2']] > > + for ovmf in self.ovmf_bios: > > + format = ovmf.rsplit('.', 1)[-1] > > + self.qemu_opt += ' -drive if=pflash,format=%s,file=%s' % > > (format, ovmf) > > if self.ovmf_bios: > > - format = self.ovmf_bios.rsplit('.', 1)[-1] > > - self.qemu_opt += ' -drive if=pflash,format=%s,file=%s' % > > (format, self.ovmf_bios) > > # OVMF only supports normal VGA, i.e. we need to override a > > -vga vmware > > # that gets added for example for normal qemux86. > > self.qemu_opt += ' -vga std' > > > > > I think this solution looks good as having separate file does not pose > an extra hassle in the user: the recipe builds all that is needed and > runqemu takes all that it needs. But not automatically. "runqemu ovmf" would expand "ovmf" to tmp/deploy/images/.../ovmf.qcow2 and thus use the combined file while "runqemu ovmf.code ovmf.vars" tells runqemu that it is supposed to use two flash drives, one with the code and one with the variables. > If in the future people shows an > interest in having unified images, maybe that can be added as another > PACKAGECONFIG? As the unified code+vars is slightly easier to use, I'd prefer to keep it around and just offer both. That's not such a big deal in terms of performance and disk usage. -- 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. -- _______________________________________________ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core