From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v4 05/14] libxl: Load guest BIOS from file Date: Wed, 16 Mar 2016 10:27:25 +0100 Message-ID: <1458120445.3102.805.camel@citrix.com> References: <1457978150-27201-1-git-send-email-anthony.perard@citrix.com> <1457978150-27201-6-git-send-email-anthony.perard@citrix.com> <20160316005310.GB29696@char.us.oracle.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2638358254350847394==" Return-path: In-Reply-To: <20160316005310.GB29696@char.us.oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Konrad Rzeszutek Wilk , Anthony PERARD Cc: xen-devel@lists.xen.org, Wei Liu , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org --===============2638358254350847394== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-j9OOkyfXl223DEySoSnT" --=-j9OOkyfXl223DEySoSnT Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, 2016-03-15 at 20:53 -0400, Konrad Rzeszutek Wilk wrote: > On Mon, Mar 14, 2016 at 05:55:40PM +0000, Anthony PERARD wrote: > >=20 > > --- a/docs/man/xl.cfg.pod.5 > > +++ b/docs/man/xl.cfg.pod.5 > > @@ -1121,6 +1121,15 @@ Requires device_model_version=3Dqemu-xen. > > =C2=A0 > > =C2=A0=3Dback > > =C2=A0 > > +=3Ditem B > Perhaps 'bios_override_path' ? >=20 Or 'bios_path_override' ? Wow, a French, an Italian and a Polish arguing about best looking English-ish words... This must be a (bad) joke! :-P :-P=C2=A0 > > + > > +Override the path to the blob to be used as BIOS. The blob > > provided here MUST > Perhaps: >=20 > Override the path to search for the B? >=20 AFAIUI, B does not contain an exact file name, but the indication of what kind of BIOS we want. Therefore, referencing it directly like this may be confusing (IMHO). So, maybe something like: "Override the path at which to look for the BIOS binary. Such binary MUST be consistent with what has been specified in B." > Or is this the full path including the name?? In which case should it > mention that B is overriden? >=20 If that is the case, indeed. > > --- a/tools/libxl/libxl_dom.c > > +++ b/tools/libxl/libxl_dom.c > > @@ -862,6 +862,38 @@ err: > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return ret; > > =C2=A0} > > =C2=A0 > > +static int libxl__load_hvm_firmware_module(libxl__gc *gc, > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0const char *filename, > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0const char *what, > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct > > xc_hvm_firmware_module *m) > > +{ > > +=C2=A0=C2=A0=C2=A0=C2=A0int datalen =3D 0; > > +=C2=A0=C2=A0=C2=A0=C2=A0void *data =3D NULL; > > +=C2=A0=C2=A0=C2=A0=C2=A0int e; > That is interesting. >=20 > The CODING_STYLE in tools/libxl says: >=20 > =C2=A036=C2=A0=C2=A0=C2=A0int rc;=C2=A0=C2=A0=C2=A0=C2=A0/* a libxl error= code - and not anything else > */=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 > =C2=A037=C2=A0=C2=A0=C2=A0int r;=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0/* the retu= rn value from a system call (or libxc > call) */=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 > =C2=A038=C2=A0=C2=A0=C2=A0bool ok;=C2=A0=C2=A0=C2=A0/* the success return= value from a boolean function > */=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 >=20 > And libxl_read_file_contents is quite weird. It does return an normal > errno value, so .. one could say it should be 'r'? But the existing > users > of this are either 'e','ret' or 'rc'. 'rc' is not good, and 'ret' > is 'rc' is clearly wrong. >=20 > How confusing. >=20 > Ian, Wei, maybe you could clarify please? >=20 Hehe... I'd also go for 'r', and call the (or at least some of the) existing users not codying style compliant, but it's indeed a funny case, and we should hear from maintainers what they prefer. :-) Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-j9OOkyfXl223DEySoSnT Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEABECAAYFAlbpJv0ACgkQk4XaBE3IOsSwCQCgqXDAkbvEVBEgAJOYgkgtrh98 AK0AmQGjHRRgGTrUXdW6yjJCHy8cgJ6S =Eu9J -----END PGP SIGNATURE----- --=-j9OOkyfXl223DEySoSnT-- --===============2638358254350847394== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============2638358254350847394==--