[libvirt] [PATCH v4] virsh: add [--domain DOMAIN] option to domxml-to-native DOMAIN COMMAND

[Julio Faracco]

Hi Dan,

Have you tested the --xml argument as VSH_OT_ARGV?
     {.name = "domain",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_STRING,
      .flags = VSH_OFLAG_REQ_OPT,
      .help = N_("domain name, id or uuid")
     },
     {.name = "xml",
-     .type = VSH_OT_DATA,
+     .type = VSH_OT_ARGV,
      .help = N_("xml data file to export from")

Check the command 'domfsthaw', option 'mountpoint'.
This is the best approach that I got until now.

2017-06-26 17:21 GMT-03:00 Dan <srwx4096 at gmail.com>:
> On Sun, Jun 25, 2017 at 08:33:39AM -0400, John Ferlan wrote:
>>
>>
>> On 06/24/2017 08:05 PM, Julio Faracco wrote:
>> > My apologies. I didn't read the function logic.
>> > The second error was introduced by my "fix".
>> > The error with help parameter is happening.
>> >
>> > If --domain is required, I believe that moving to STRING
>> > is enough. As I said, VSH_OT_DATA requires
>> > VSH_OFLAG_REQ. See vsh.c line 746.
>> >
>> >      {.name = "domain",
>> > -     .type = VSH_OT_DATA,
>> > +     .type = VSH_OT_STRING,
>> >       .flags = VSH_OFLAG_REQ_OPT,
>> >       .help = N_("domain name, id or uuid")
>> >      },
>> >      {.name = "xml",
>> > -     .type = VSH_OT_DATA,
>> > +     .type = VSH_OT_STRING,
>> >       .help = N_("xml data file to export from")
>> >
>>
>> ugh... Could you send a patch please. I do recall at one point wondering
>> what the downside of removing VSH_OFLAG_REQ from "xml" .flags was, no I
>> know. Of course I probably won't remember the next time either
>>
>> Tks, -
>>
>> John
>>
> I think we may need to modify vshCmddefHelp() in vsh.c to really make it
> look like as expected. For example, in some case, the SYNOPSIS part also
> displayed incorrectly.
>
> SYNOPSIS
>  domxml-to-native <format> <xml> <domain>
>
>
> Instead, the following is similar to what we want:
>
> SYNOPSIS-
>  domxml-to-native <format> { [--xml] <xml> | --domain <domain> }
>
> Current implementation of vshCmddefHelp() does not handle this
> "complicated" logic. The question is whether we want to modify it or
> whether we want to spend the effort modifying it?
>
> Dan
>> > 2017-06-24 17:54 GMT-03:00 Julio Faracco <jcfaracco at gmail.com>:
>> >> Hi guys,
>> >>
>> >> I updated the source this weekend, I missed the ability of calling help.
>> >> virsh # domxml-to-native --help
>> >>   NAME
>> >>     domxml-to-native - Convert domain XML to native config
>> >>
>> >>   SYNOPSIS
>> >>     domxml-to-native <format> [<domain>] [<xml>]
>> >>
>> >>   DESCRIPTION
>> >>     Convert domain XML config to a native guest configuration format.
>> >>
>> >>   OPTIONS
>> >>     [--format] <string>  target config data type format
>> >> error: internal error: bad options in command: 'domxml-to-native'
>> >>
>> >>
>> >> This is why:
>> >> --- a/tools/virsh-domain.c
>> >> +++ b/tools/virsh-domain.c
>> >> @@ -9858,11 +9858,11 @@ static const vshCmdOptDef opts_domxmltonative[] = {
>> >>      },
>> >>      {.name = "domain",
>> >>       .type = VSH_OT_DATA,
>> >> -     .flags = VSH_OFLAG_REQ_OPT,
>> >> +     .flags = VSH_OFLAG_REQ,
>> >>       .help = N_("domain name, id or uuid")
>> >>      },
>> >>      {.name = "xml",
>> >> -     .type = VSH_OT_DATA,
>> >> +     .type = VSH_OT_STRING,
>> >>       .help = N_("xml data file to export from")
>> >>      },
>> >>      {.name = NULL}
>> >>
>> >> VSH_OT_DATA requires VSH_OFLAG_REQ.
>> >> So, since XML is not required...
>> >> This diff fits this case. But I'm still confused.
>> >> Because I cannot check my XML files right now.
>> >>
>> >> virsh # domxml-to-native qemu-argv /home/julio/WINDOWS_7.xml
>> >> error: failed to get domain '/home/julio/WINDOWS_7.xml'
>> >> error: Domain not found: no domain with matching name
>> >> '/home/julio/WINDOWS_7.xml'
>> >>
>> >> 2017-06-23 6:38 GMT-03:00 Martin Kletzander <mkletzan at redhat.com>:
>> >>> On Thu, Jun 22, 2017 at 06:21:49PM -0400, John Ferlan wrote:
>> >>>>
>> >>>>
>> >>>> [...]
>> >>>>
>> >>>>>>>>
>> >>>>>>>
>> >>>>>>> There was no change, it is an additional variable, the original one is
>> >>>>>>> below.  The number of differences would be the same, I believe.
>> >>>>>>>
>> >>>>>>
>> >>>>>> If edit the file and change "xml" to "xmlFile" and change the 3 changed
>> >>>>>> xml variable references things work... Like I said, nit, IDC if it's
>> >>>>>> changed or not...
>> >>>>>>
>> >>>>>
>> >>>>> My bad, I misread that, you're right.
>> >>>>
>> >>>>
>> >>>> In order to "close" on this, if a squash the attach patch does that work
>> >>>> for everyone?
>> >>>>
>> >>>> John
>> >>>
>> >>>
>> >>> WFM
>> >>>
>> >>> Reviewed-by: Martin Kletzander <mkletzan at redhat.com>
>> >>>
>> >>> --
>> >>> libvir-list mailing list
>> >>> libvir-list at redhat.com
>> >>> https://www.redhat.com/mailman/listinfo/libvir-list