Re: [libvirt] [PATCH v2 2/6] bhyve: add bhyveDomainDefNeedsISAController helper

On Sun, Feb 17, 2019 at 05:04:01PM +0400, Roman Bogorodskiy wrote: >Add a bhyveDomainDefNeedsISAController() helper function >which by domain configuration determines whether LPC controller is >required or not. > >Signed-off-by: Roman Bogorodskiy <bogorodskiy(a)gmail.com&gt; >--- > src/bhyve/bhyve_command.c | 5 +---- > src/bhyve/bhyve_domain.c | 10 ++++++++++ > src/bhyve/bhyve_domain.h | 2 ++ > 3 files changed, 13 insertions(+), 4 deletions(-) > >diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c >index 1f215dac08..f49dc77118 100644 >--- a/src/bhyve/bhyve_command.c >+++ b/src/bhyve/bhyve_command.c >@@ -460,7 +460,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, > * vm0 > */ > size_t i; >- bool add_lpc = false; > int nusbcontrollers = 0; > unsigned int nvcpus = virDomainDefGetVcpus(def); > >@@ -549,7 +548,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, > if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_LPC_BOOTROM)) { > virCommandAddArg(cmd, "-l"); > virCommandAddArgFormat(cmd, "bootrom,%s", def->os.loader->path); >- add_lpc = true; > } else { > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > _("Installed bhyve binary does not support " >@@ -613,7 +611,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, > if (bhyveBuildGraphicsArgStr(def, def->graphics[0], def->videos[0], > conn, cmd, dryRun) < 0) > goto error; >- add_lpc = true; > } else { > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > _("Multiple graphics devices are not supported")); >@@ -621,7 +618,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, > } > } > >- if (add_lpc || def->nserials) >+ if (bhyveDomainDefNeedsISAController(def)) > bhyveBuildLPCArgStr(def, cmd); > > if (bhyveBuildConsoleArgStr(def, cmd) < 0) >diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c >index 79cf103d28..e476ef7e7d 100644 >--- a/src/bhyve/bhyve_domain.c >+++ b/src/bhyve/bhyve_domain.c >@@ -61,6 +61,16 @@ virDomainXMLPrivateDataCallbacks virBhyveDriverPrivateDataCallbacks = { > .free = bhyveDomainObjPrivateFree, > }; > >+bool >+bhyveDomainDefNeedsISAController(virDomainDefPtr def) >+{ >+ if ((def->os.bootloader == NULL && def->os.loader) || >+ (def->nconsoles || def->nserials) || (def->ngraphics && def->nvideos)) >+ return true; Where did nconsoles come from? Also, since it's a separate function, you can use return more: if (def->os.bootloader == NULL && def->os.loader) return true; if (def->nserials) return true; >+ else >+ return false; >+} >+ > static int > bhyveDomainDefPostParse(virDomainDefPtr def, > virCapsPtr caps ATTRIBUTE_UNUSED, With nconsoles removed: Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; Jano