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

[Roman Bogorodskiy]

  Ján Tomko wrote:

> 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 at gmail.com>
> >---
> > 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 at redhat.com>
> 
> Jano

Thanks, merged this one with suggestions applied, the rest will go into
v3.

Roman Bogorodskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190224/8449b118/attachment-0001.sig>