Michal Privoznik wrote:
> On 2/4/21 4:47 PM, Roman Bogorodskiy wrote:
> > Currently, requesting domain capabilities fails when the specified
> > emulator binary does not equal to "/usr/sbin/bhyve". Relax this check
> > to allow any path with basename "bhyve", as it's a common case when
> > binary is specified without an absolute path.
> >
> > Signed-off-by: Roman Bogorodskiy <bogorodskiy@xxxxxxxxx>
> > ---
> > I'm not sure how useful is this check in general: at this point we don't
> > use the user specified emulator value anyway, just use BHYVE constant.
> > Probably it would be better to just drop the entire "else" block?
>
> Yes, I was just about to suggest that. We don't do any check like this
> for QEMU driver. Just execute user provided binary (with sume arguments
> appended to get QMP monitor) and query caps.
>
> Looking into bhyve driver code though, it doesn't use <emulator/> from
> domain XML, does it? What I'm getting at is that there is no way for a
> user to specify different binary to run than /usr/sbin/bhyve. So it
> doesn't really make sense to provide emulatorbin at all.
Yes, currently we just use BHYVE (from config.h) to build commands.
In general, I've just realized that it's a little bit messy right now.
Even if we switch command line builder to use user specified value,
this will not work properly because we still use virFindFileInPath("bhyve");
to populate driver->bhyvecaps.
I guess the right way would be to move most of the code from
virBhyveProbeCaps() to virBhyveDomainCapsFill() and try to use the user
specified binary.
> Since I can argue both ways, merge this patch or just remove the block
> completely.
I'll just remove the block then.
Thanks,
> Reviewed-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
>
> Michal
>
Roman Bogorodskiy
Attachment:
signature.asc
Description: PGP signature
