Re: [PATCH for 1.2.7 3/8] virsh: expose virConnectGetDomainCapabilities

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 06/30/2014 11:31 AM, Michal Privoznik wrote:
> The API is exposed under 'domcapabilities' command. Currently, with
> the variety of drivers that libvirt supports, none of the command
> arguments is obligatory, but all are optional instead.
> 
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
>  tools/virsh-host.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tools/virsh.pod    | 16 +++++++++++
>  2 files changed, 100 insertions(+)
> 
> diff --git a/tools/virsh-host.c b/tools/virsh-host.c
> index 734f1a8..a1d8465 100644
> --- a/tools/virsh-host.c
> +++ b/tools/virsh-host.c
> @@ -69,6 +69,84 @@ cmdCapabilities(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
>  }
>  
>  /*
> + * "domcapabilities" command
> + */
> +static const vshCmdInfo info_domcapabilities[] = {
> +    {.name = "help",
> +     .data = N_("domain capabilities")
> +    },
> +    {.name = "desc",
> +     .data = N_("Returns capabilities of emulator with respect to host and libvirt.")
> +    },
> +    {.name = NULL}
> +};
> +
> +static const vshCmdOptDef opts_domcapabilities[] = {
> +    {.name = "virttype",
> +     .type = VSH_OT_STRING,
> +     .help = N_("virtualization type (/domain/@type)"),
> +    },
> +    {.name = "emulatorbin",
> +     .type = VSH_OT_STRING,
> +     .help = N_("path to emulator binary (/domain/devices/emulator)"),
> +    },
> +    {.name = "arch",
> +     .type = VSH_OT_STRING,
> +     .help = N_("domain architecture (/domain/os/type/@arch)"),
> +    },
> +    {.name = "machine",
> +     .type = VSH_OT_STRING,
> +     .help = N_("machine type (/domain/os/type/@machine)"),
> +    },
> +    {.name = NULL}
> +};
> +
> +static bool
> +cmdDomCapabilities(vshControl *ctl, const vshCmd *cmd)
> +{
> +    bool ret = false;
> +    char *caps;
> +    const char *virttype = NULL;
> +    const char *emulatorbin = NULL;
> +    const char *arch = NULL;
> +    const char *machine = NULL;
> +    const unsigned int flags = 0; /* No flags so far */
> +
> +    if (vshCommandOptString(cmd, "virttype", &virttype) < 0) {
> +        vshError(ctl, "%s", _("ble"));
> +        goto cleanup;
> +    }
> +
> +    if (vshCommandOptString(cmd, "emulatorbin", &emulatorbin) < 0) {
> +        vshError(ctl, "%s", _("ble"));
> +        goto cleanup;
> +    }
> +
> +    if (vshCommandOptString(cmd, "arch", &arch) < 0) {
> +        vshError(ctl, "%s", _("ble"));
> +        goto cleanup;
> +    }
> +
> +    if (vshCommandOptString(cmd, "machine", &machine) < 0) {
> +        vshError(ctl, "%s", _("ble"));
> +        goto cleanup;
> +    }

None of the above have realistic error messages. If they're not found on
the command line, then how should things proceed?

Although the code in the qemu driver seems to only care that virttype is
provided. The qemu driver also requires arch or emulatorbin to be
provided in order to fetch machine (and other caps).

> +
> +    caps = virConnectGetDomainCapabilities(ctl->conn, emulatorbin,
> +                                           arch, machine, virttype, flags);
> +    if (!caps) {
> +        vshError(ctl, "%s", _("failed to get emulator capabilities"));
> +        goto cleanup;
> +    }
> +
> +    vshPrint(ctl, "%s\n", caps);
> +    ret = true;
> + cleanup:
> +    VIR_FREE(caps);
> +    return ret;
> +}
> +
> +/*
>   * "freecell" command
>   */
>  static const vshCmdInfo info_freecell[] = {
> @@ -1131,6 +1209,12 @@ const vshCmdDef hostAndHypervisorCmds[] = {
>       .info = info_cpu_models,
>       .flags = 0
>      },
> +    {.name = "domcapabilities",
> +     .handler = cmdDomCapabilities,
> +     .opts = opts_domcapabilities,
> +     .info = info_domcapabilities,
> +     .flags = 0
> +    },
>      {.name = "freecell",
>       .handler = cmdFreecell,
>       .opts = opts_freecell,
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index b248c9a..b37a2be 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -350,6 +350,22 @@ description see:
>    L<http://libvirt.org/formatcaps.html>
>  The XML also show the NUMA topology information if available.
>  
> +=item B<domcapabilities> [I<virttype>] [I<emulatorbin>]
> +[I<arch>] [I<machine>]
> +
> +Print an XML document describing the capabilities of the
> +hypervisor we are currently connected to. This may be useful if
> +you intend to create a new domain and are curious if for instance
> +should use VFIO or legacy KVM device passthrough. The I<virttype>
> +specifies the virtualization used (the domain XML counterpart is
> +the 'type' attribute of the <domain/> top level element). Then,
> +the I<emulatorbin> specifies the path to the emulator (this is
> +same as <emulator> element in the domain XML). Then, the I<arch>
> +argument sets the domain architecture (exposed under
> +/domain/os/type/@arch attribute). Then at last I<machine>
> +overrides the default machine for the emulator (can be found in
> +domain XML under /domain/os/type).
> +

>From just the description, I would think I could get all the caps for
the domain of the hypervisor that virsh is connected to.  Although I
still have to provide the virttype (which should be part of the
connection right)?  I guess this goes along with my comments from patch
1 about not having a clear enough picture yet to know the best way to
describe the virsh.pod fields.

Once that picture is clearer I figure it'll be easier for me at least to
provide some text advice here...  For starters though, the "Then, the"
for each option is superfluous - just go with "The"... Also, "...and are
curious if for instance should use VFIO..." doesn't read well or convey
(at least to me) what to expect.

John

>  =item B<inject-nmi> I<domain>
>  
>  Inject NMI to the guest.
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]