Re: [PATCH 05/11] tools: add option inactive to nodedev-dumpxml

On 1/31/24 22:34, Jonathon Jongsma wrote: > On 1/19/24 10:38 AM, Boris Fiuczynski wrote: >> Allow to dump the XML of the persisted mdev when the mdev has been >> started instead of the current state only. >> >> Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; >> --- >>   docs/manpages/virsh.rst |  7 +++++-- >>   tools/virsh-nodedev.c   | 15 ++++++++++++++- >>   2 files changed, 19 insertions(+), 3 deletions(-) >> >> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst >> index ed1027e133..3a814dccd2 100644 >> --- a/docs/manpages/virsh.rst >> +++ b/docs/manpages/virsh.rst >> @@ -5422,14 +5422,17 @@ nodedev-dumpxml >>   :: >> -   nodedev-dumpxml [--xpath EXPRESSION] [--wrap] device >> +   nodedev-dumpxml [--inactive] [--xpath EXPRESSION] [--wrap] device >>   Dump a <device> XML representation for the given node device, >> including >>   such information as the device name, which bus owns the device, the >>   vendor and product id, and any capabilities of the device usable by >>   libvirt (such as whether device reset is supported). *device* can >>   be either device name or wwn pair in "wwnn,wwpn" format (only works >> -for HBA). >> +for HBA). An additional option affecting the XML dump may be >> +used. *--inactive* tells virsh to dump the node device configuration >> +that will be used on next start of the node device as opposed to the >> +current node device configuration. >>   If the **--xpath** argument provides an XPath expression, it will be >>   evaluated against the output XML and only those matching nodes will >> diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c >> index fb38fd7fcc..54e192d619 100644 >> --- a/tools/virsh-nodedev.c >> +++ b/tools/virsh-nodedev.c >> @@ -575,6 +575,10 @@ static const vshCmdOptDef >> opts_node_device_dumpxml[] = { >>        .help = N_("device name or wwn pair in 'wwnn,wwpn' format"), >>        .completer = virshNodeDeviceNameCompleter, >>       }, >> +    {.name = "inactive", >> +     .type = VSH_OT_BOOL, >> +     .help = N_("show inactive defined XML"), >> +    }, >>       {.name = "xpath", >>        .type = VSH_OT_STRING, >>        .flags = VSH_OFLAG_REQ_OPT, >> @@ -594,6 +598,7 @@ cmdNodeDeviceDumpXML(vshControl *ctl, const >> vshCmd *cmd) >>       g_autoptr(virshNodeDevice) device = NULL; >>       g_autofree char *xml = NULL; >>       const char *device_value = NULL; >> +    unsigned int flags = 0; >>       bool wrap = vshCommandOptBool(cmd, "wrap"); >>       const char *xpath = NULL; >> @@ -608,7 +613,15 @@ cmdNodeDeviceDumpXML(vshControl *ctl, const >> vshCmd *cmd) >>       if (!device) >>           return false; >> -    if (!(xml = virNodeDeviceGetXMLDesc(device, 0))) >> +    if (vshCommandOptBool(cmd, "inactive")) { >> +        flags |= VIR_NODE_DEVICE_GET_XML_DESC_INACTIVE; >> +        if (!virNodeDeviceIsPersistent(device)) { >> +            vshError(ctl, _("Node device '%1$s' is not persistent"), >> device_value); >> +            return false; >> +        } > > I believe you've already handled this within virNodeDeviceGetXMLDesc() > in patch 4. There shouldn't be any need to duplicate that check here. > Without the check in the client the error message looks like this: # virsh nodedev-dumpxml --inactive mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_0_0_0008 error: Requested operation is not valid: node device 'mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_0_0_0008' is not persistent I added the additional check in the virsh client to create a user friendlier message looking like this: # virsh nodedev-dumpxml --inactive mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_0_0_0008 error: Node device 'mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_0_0_0008' is not persistent Do you want it removed?

>> +    } >> + >> +    if (!(xml = virNodeDeviceGetXMLDesc(device, flags))) >>           return false; >>       return virshDumpXML(ctl, xml, "node-device", xpath, wrap); > > Reviewed-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; Thanks > _______________________________________________ > Devel mailing list -- devel(a)lists.libvirt.org > To unsubscribe send an email to devel-leave(a)lists.libvirt.org