Re: [PATCH v8 05/13] libxl: Load guest BIOS from file

[Wei Liu <wei.liu2@citrix.com>]

On Tue, Aug 23, 2016 at 04:00:24PM -0400, Doug Goldstein wrote:
> On 8/18/16 10:13 AM, Wei Liu wrote:
> 
> >  
> > +    if (info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> > +        if (info->u.hvm.system_firmware) {
> > +            bios_filename = info->u.hvm.system_firmware;
> > +        } else {
> > +            switch (info->u.hvm.bios) {
> > +            case LIBXL_BIOS_TYPE_SEABIOS:
> > +                bios_filename = libxl__seabios_path();
> > +                break;
> > +            case LIBXL_BIOS_TYPE_OVMF:
> > +                bios_filename = libxl__ovmf_path();
> > +                break;
> > +            case LIBXL_BIOS_TYPE_ROMBIOS:
> > +            default:
> > +                abort();
> 
> Wei,
> 
> Please consider another solution. I've been trying to use libxl from
> Rust and the calls to abort() are just really hard to deal with. I know
> libxl has 50+ calls currently but let's work on reducing these as much
> as possible so that its possible to consume libxl in other languages.
> 
> abort() is just bad form for libraries because you don't give the caller
> the ability to catch the error and handle it appropriately (which could
> be as simple as displaying it to the user or potentially work around the
> issue.
> 
> I know you and Anthony have gone through 8 revisions but please consider
> changing this. I'm sorry for not speaking up sooner.

As said, the abort() in internal function marks an impossible situation
to get into -- much like BUG_ON in hypervisor. I would expect the error
to be handled in some other places in libxl. In this particular
instance, I haven't checked, but if there is no such check elsewhere, I
will either fix it here or somewhere else appropriate.

Furthermore, I'm not averse to systematically removing abort(), but I
would like to at least have a rough idea how upper layer would want to
handle that, and what is the implication on other parts of libxl.

Wei.

> -- 
> Doug Goldstein
> 




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel