[libvirt] [PATCH console-proxy 0/6] libvirt-go-xml and libvirt-console-proxy ?

Hello Daniel, I regularly visit your blog and stumbled over <https://www.berrange.com/posts/2017/01/05/announce-new-libvirt-project-go-xml-parser-model/>, which seems to contains some small errors - I'm a "go"-newby, so I had to spend some time to understand your example and to make it go:
import ( libvirt "github.com/libvirt/libvirt-go" libvirtxml "github.com/libvirt/libvirt-go-xml" "encoding/xml" "fmt" )
conn, err := libvirt.NewConnect("qemu:///system") dom := conn.LookupDomainByName("demo")
This also returns a tuple (dom, err), so should look like: dom, err := conn.LookupDomainByName("demo")
xmldoc, err := dom.GetXMLDesc(0)
domcfg := &libvirtxml.Domain{} err := xml.Unmarshal([]byte(xmldocC), domcfg)
This fails as "err" is already defined. Should be '=' instead of ':=': err = xml.Unmarshal([]byte(xmldocC), domcfg)
fmt.Printf("Virt type %s", domcfg.Type)
Maybe add an "\n" at the end? And maybe also add the wrapper
package main ... func main() { ... } to make the example complete?
Second is <https://www.berrange.com/posts/2017/01/26/announce-new-libvirt-console-proxy-project/>: Do you (or someone else) still work on libvirt-console-proxy? It no longer compiles after several API changed in libvirt-go-xml and uuid. I have attached several patches, which at least allow compiling again, I only tested is briefly. Philipp Philipp Hahn (6): Adapt to API change for dom.Devices.Graphics Adapt to uuid API change Adapt to API change for dom.Devices.{Serials,Console} virtconsoleresolveradm: Require sub-command virtconsoleresolveradm: Simplify argument parsing virtconsoleresolveradm: Fix -c handling cmd/virtconsoleresolveradm/cmd/disable.go | 6 +----- cmd/virtconsoleresolveradm/cmd/enable.go | 20 ++++++----------- cmd/virtconsoleresolveradm/cmd/root.go | 3 ++- pkg/resolver/server.go | 26 ++++++++++++++++------- 4 files changed, 28 insertions(+), 27 deletions(-) -- 2.20.1

Follow
commit 4772b681e87a8d0dcee011b8e43813e851e4f934 Author: Daniel P. Berrange <berrange@redhat.com> Date: Fri Nov 24 16:15:30 2017 +0000
Add remaining graphics configuration options
Signed-off-by: Philipp Hahn <hahn@univention.de> --- cmd/virtconsoleresolveradm/cmd/enable.go | 6 ++---- pkg/resolver/server.go | 26 ++++++++++++++++-------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/cmd/virtconsoleresolveradm/cmd/enable.go b/cmd/virtconsoleresolveradm/cmd/enable.go index 04ca80b..dd95e9e 100644 --- a/cmd/virtconsoleresolveradm/cmd/enable.go +++ b/cmd/virtconsoleresolveradm/cmd/enable.go @@ -164,15 +164,13 @@ func doEnable(cmd *cobra.Command, args []string) { meta = &resolver.ConsoleServerProxyMetadata{} for _, graphics := range domcfg.Devices.Graphics { - switch graphics.Type { - case "spice": + if graphics.Spice != nil { meta.Consoles = append(meta.Consoles, createConsole("spice", 0, conn, domname, domuuid)) - case "vnc": + } else if graphics.VNC != nil { meta.Consoles = append(meta.Consoles, createConsole("vnc", 0, conn, domname, domuuid)) } - } for idx, chardev := range domcfg.Devices.Serials { diff --git a/pkg/resolver/server.go b/pkg/resolver/server.go index 115d75d..eefdad7 100644 --- a/pkg/resolver/server.go +++ b/pkg/resolver/server.go @@ -86,13 +86,19 @@ type ConsoleServer struct { const tokenpath = "/consoleresolver/token/" +func isListenAddress(listen string) bool { + return listen != "" && listen != "0.0.0.0" && listen != "::" +} + func getListener(dom libvirtxml.Domain, gtype string, insecure bool, consoleHost, defaultHost string) (string, error) { if dom.Devices == nil { return "", errors.New("No devices present") } for _, graphics := range dom.Devices.Graphics { - if graphics.Type != gtype { + if gtype == "vnc" && graphics.VNC != nil { + } else if gtype == "spice" && graphics.Spice != nil { + } else { continue } @@ -100,21 +106,25 @@ func getListener(dom libvirtxml.Domain, gtype string, insecure bool, consoleHost if consoleHost != "" { host = consoleHost } else { - if graphics.Listen != "" && graphics.Listen != "0.0.0.0" && graphics.Listen != "::" { - host = graphics.Listen + if graphics.VNC != nil && isListenAddress(graphics.VNC.Listen) { + host = graphics.VNC.Listen + } else if graphics.Spice != nil && isListenAddress(graphics.Spice.Listen) { + host = graphics.Spice.Listen } else { host = defaultHost } } var port int - if graphics.Type == "spice" && !insecure { - port = graphics.TLSPort - } else { - port = graphics.Port + if graphics.Spice != nil && !insecure { + port = graphics.Spice.TLSPort + } else if graphics.Spice != nil { + port = graphics.Spice.Port + } else if graphics.VNC != nil { + port = graphics.VNC.Port } glog.V(1).Infof("Got port %d\n", port) - if graphics.Port == 0 || graphics.Port == -1 { + if port == 0 || port == -1 { return "", errors.New("Missing port for graphics") } -- 2.20.1

commit 0ef6afb2f6cdd6cdaeee3885a95099c63f18fc8c Author: Maxim Bublis <b@codemonkey.ru> Date: Wed Jan 3 16:02:28 2018 +0000
Return enthropy errors from UUID generation.
This commit changes signature for `NewV1`, `NewV2` and `NewV4` functions which from now will return `(UUID, error)` instead of `UUID`. To emulate old behavior of panicking on enthropy errors one can wrap a call into `Must` helper similar to: ``` u := uuid.Must(uuid.NewV4())
Signed-off-by: Philipp Hahn <hahn@univention.de> --- cmd/virtconsoleresolveradm/cmd/enable.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/virtconsoleresolveradm/cmd/enable.go b/cmd/virtconsoleresolveradm/cmd/enable.go index dd95e9e..1a4a12d 100644 --- a/cmd/virtconsoleresolveradm/cmd/enable.go +++ b/cmd/virtconsoleresolveradm/cmd/enable.go @@ -49,8 +49,8 @@ var ( ) func createConsole(ctype string, index int, conn *libvirt.Connect, domname, domuuid string) resolver.ConsoleServerProxyMetadataConsole { - tokenID := uuid.NewV4() - tokenValue := uuid.NewV4() + tokenID := uuid.Must(uuid.NewV4()) + tokenValue := uuid.Must(uuid.NewV4()) console := resolver.ConsoleServerProxyMetadataConsole{ Token: tokenID.String(), Type: ctype, -- 2.20.1

Follow
commit d44fdf9badbb798949ab9f82ef516a2fd36d8b55 Author: Daniel P. Berrange <berrange@redhat.com> Date: Tue Nov 28 11:50:06 2017 +0000
Generalize protocol handling for chardev sources
Signed-off-by: Philipp Hahn <hahn@univention.de> --- cmd/virtconsoleresolveradm/cmd/enable.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/virtconsoleresolveradm/cmd/enable.go b/cmd/virtconsoleresolveradm/cmd/enable.go index 1a4a12d..ebad497 100644 --- a/cmd/virtconsoleresolveradm/cmd/enable.go +++ b/cmd/virtconsoleresolveradm/cmd/enable.go @@ -174,14 +174,14 @@ func doEnable(cmd *cobra.Command, args []string) { } for idx, chardev := range domcfg.Devices.Serials { - if chardev.Type == "tcp" { + if chardev.Source.TCP != nil { meta.Consoles = append(meta.Consoles, createConsole("serial", idx, conn, domname, domuuid)) } } for idx, chardev := range domcfg.Devices.Consoles { - if chardev.Type == "tcp" { + if chardev.Source.TCP != nil { meta.Consoles = append(meta.Consoles, createConsole("console", idx, conn, domname, domuuid)) } -- 2.20.1

Signed-off-by: Philipp Hahn <hahn@univention.de> --- cmd/virtconsoleresolveradm/cmd/root.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/virtconsoleresolveradm/cmd/root.go b/cmd/virtconsoleresolveradm/cmd/root.go index 76e1203..7aaca47 100644 --- a/cmd/virtconsoleresolveradm/cmd/root.go +++ b/cmd/virtconsoleresolveradm/cmd/root.go @@ -34,6 +34,7 @@ var RootCmd = &cobra.Command{ Short: "Libvirt console resolver admin", Long: "Libvirt console resolver administration tool", Run: func(cmd *cobra.Command, args []string) {}, + Args: cobra.MinimumNArgs(1), } var ( -- 2.20.1

Use ExactArgs(1) to check for exactly one positional argument. <https://github.com/spf13/cobra#positional-and-custom-arguments> Signed-off-by: Philipp Hahn <hahn@univention.de> --- cmd/virtconsoleresolveradm/cmd/disable.go | 6 +----- cmd/virtconsoleresolveradm/cmd/enable.go | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/cmd/virtconsoleresolveradm/cmd/disable.go b/cmd/virtconsoleresolveradm/cmd/disable.go index 2d41874..aab3047 100644 --- a/cmd/virtconsoleresolveradm/cmd/disable.go +++ b/cmd/virtconsoleresolveradm/cmd/disable.go @@ -40,14 +40,10 @@ var disableCmd = &cobra.Command{ Short: "Disable consoles for a domain", Long: "Disable access to consoles for a domain", Run: doDisable, + Args: cobra.ExactArgs(1), } func doDisable(cmd *cobra.Command, args []string) { - if len(args) != 1 { - fmt.Fprintln(os.Stderr, "Missing domain name/uuid") - os.Exit(1) - } - conn, err := libvirt.NewConnect(connect) if err != nil { fmt.Fprintf(os.Stderr, "Cannot connect to hypervisor '%s': %s\n", diff --git a/cmd/virtconsoleresolveradm/cmd/enable.go b/cmd/virtconsoleresolveradm/cmd/enable.go index ebad497..49897ef 100644 --- a/cmd/virtconsoleresolveradm/cmd/enable.go +++ b/cmd/virtconsoleresolveradm/cmd/enable.go @@ -43,6 +43,7 @@ var ( Short: "Enable consoles for a domain", Long: "Enable consoles for a domain", Run: doEnable, + Args: cobra.ExactArgs(1), } insecure *bool host *string @@ -99,11 +100,6 @@ func createConsole(ctype string, index int, conn *libvirt.Connect, domname, domu } func doEnable(cmd *cobra.Command, args []string) { - if len(args) != 1 { - fmt.Fprintln(os.Stderr, "Missing domain name/uuid") - os.Exit(1) - } - conn, err := libvirt.NewConnect(connect) if err != nil { fmt.Fprintf(os.Stderr, "Cannot connect to hypervisor '%s': %s\n", -- 2.20.1

-c / --connect <Qemu-URI> does not work:
$ build/virtconsoleresolveradm -c qemu:///session enable $VM Error: unknown shorthand flag: 'c' in -c
<https://github.com/spf13/cobra#persistent-flags> Signed-off-by: Philipp Hahn <hahn@univention.de> --- cmd/virtconsoleresolveradm/cmd/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/virtconsoleresolveradm/cmd/root.go b/cmd/virtconsoleresolveradm/cmd/root.go index 7aaca47..d4001e4 100644 --- a/cmd/virtconsoleresolveradm/cmd/root.go +++ b/cmd/virtconsoleresolveradm/cmd/root.go @@ -42,5 +42,5 @@ var ( ) func init() { - RootCmd.Flags().StringVarP(&connect, "connect", "c", "qemu:///system", "Libvirt connection URI") + RootCmd.PersistentFlags().StringVarP(&connect ,"connect", "c", "qemu:///system", "Libvirt connection URI") } -- 2.20.1

On Fri, Nov 15, 2019 at 02:05:11PM +0100, Philipp Hahn wrote:
Hello Daniel,
I regularly visit your blog and stumbled over <https://www.berrange.com/posts/2017/01/05/announce-new-libvirt-project-go-xml-parser-model/>, which seems to contains some small errors - I'm a "go"-newby, so I had to spend some time to understand your example and to make it go:
import ( libvirt "github.com/libvirt/libvirt-go" libvirtxml "github.com/libvirt/libvirt-go-xml" "encoding/xml" "fmt" )
conn, err := libvirt.NewConnect("qemu:///system") dom := conn.LookupDomainByName("demo")
This also returns a tuple (dom, err), so should look like:
dom, err := conn.LookupDomainByName("demo")
xmldoc, err := dom.GetXMLDesc(0)
domcfg := &libvirtxml.Domain{} err := xml.Unmarshal([]byte(xmldocC), domcfg)
This fails as "err" is already defined. Should be '=' instead of ':=':
err = xml.Unmarshal([]byte(xmldocC), domcfg)
fmt.Printf("Virt type %s", domcfg.Type)
Maybe add an "\n" at the end?
And maybe also add the wrapper
package main ... func main() { ... } to make the example complete?
Yes, all good suggestions. FYI this text from my blog is copied into the libvirt-go-xml docs if you fancy sending a fix for these: https://libvirt.org/git/?p=libvirt-go-xml.git;a=blob;f=doc.go;hb=HEAD
Second is <https://www.berrange.com/posts/2017/01/26/announce-new-libvirt-console-proxy-project/>: Do you (or someone else) still work on libvirt-console-proxy? It no longer compiles after several API changed in libvirt-go-xml and uuid. I have attached several patches, which at least allow compiling again, I only tested is briefly.
Thank you for your patches, I've applied them all, and also added support for "go mod" to replace the deprecated glide tool. It is best to consider the project dormant right now. As you can see from the git history I've not done any work on it for a while. I still think its a conceptually useful thing to have but it needs someone with time & motiviation to take it forward. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Hello, as requested here are my proposed changes from <https://www.berrange.com/posts/2017/01/05/announce-new-libvirt-project-go-xml-parser-model/> for libvirt-go-xml/doc.go. Philipp Hahn (4): doc: Handle conn.LookupDomainByName() error doc: Variable is already defined doc: Add missing newline doc: Make examples a complete program doc.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) -- 2.20.1

Signed-off-by: Philipp Hahn <hahn@univention.de> --- doc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc.go b/doc.go index 2886c79..f97f2eb 100644 --- a/doc.go +++ b/doc.go @@ -51,7 +51,7 @@ // ) // // conn, err := libvirt.NewConnect("qemu:///system") -// dom := conn.LookupDomainByName("demo") +// dom, err := conn.LookupDomainByName("demo") // xmldoc, err := dom.GetXMLDesc(0) // // domcfg := &libvirtxml.Domain{} -- 2.20.1

Signed-off-by: Philipp Hahn <hahn@univention.de> --- doc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc.go b/doc.go index f97f2eb..d1f94b2 100644 --- a/doc.go +++ b/doc.go @@ -55,7 +55,7 @@ // xmldoc, err := dom.GetXMLDesc(0) // // domcfg := &libvirtxml.Domain{} -// err := domcfg.Unmarshal(xmldoc) +// err = domcfg.Unmarshal(xmldoc) // // fmt.Printf("Virt type %s", domcfg.Type) // -- 2.20.1

Signed-off-by: Philipp Hahn <hahn@univention.de> --- doc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc.go b/doc.go index d1f94b2..c4ce0dd 100644 --- a/doc.go +++ b/doc.go @@ -57,6 +57,6 @@ // domcfg := &libvirtxml.Domain{} // err = domcfg.Unmarshal(xmldoc) // -// fmt.Printf("Virt type %s", domcfg.Type) +// fmt.Printf("Virt type %s\n", domcfg.Type) // package libvirtxml -- 2.20.1

Add `package main` and declare a `main()` function. Signed-off-by: Philipp Hahn <hahn@univention.de> --- doc.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/doc.go b/doc.go index c4ce0dd..25bc1d5 100644 --- a/doc.go +++ b/doc.go @@ -33,30 +33,38 @@ // // Example creating a domain XML document from configuration: // +// package main +// // import ( // "github.com/libvirt/libvirt-go-xml" // ) // -// domcfg := &libvirtxml.Domain{Type: "kvm", Name: "demo", -// UUID: "8f99e332-06c4-463a-9099-330fb244e1b3", -// ....} -// xmldoc, err := domcfg.Marshal() +// func main() { +// domcfg := &libvirtxml.Domain{Type: "kvm", Name: "demo", +// UUID: "8f99e332-06c4-463a-9099-330fb244e1b3", +// ....} +// xmldoc, err := domcfg.Marshal() +// } // // Example parsing a domainXML document, in combination with libvirt-go // +// package main +// // import ( // "github.com/libvirt/libvirt-go" // "github.com/libvirt/libvirt-go-xml" // "fmt" // ) // -// conn, err := libvirt.NewConnect("qemu:///system") -// dom, err := conn.LookupDomainByName("demo") -// xmldoc, err := dom.GetXMLDesc(0) +// func main() { +// conn, err := libvirt.NewConnect("qemu:///system") +// dom, err := conn.LookupDomainByName("demo") +// xmldoc, err := dom.GetXMLDesc(0) // -// domcfg := &libvirtxml.Domain{} -// err = domcfg.Unmarshal(xmldoc) +// domcfg := &libvirtxml.Domain{} +// err = domcfg.Unmarshal(xmldoc) // -// fmt.Printf("Virt type %s\n", domcfg.Type) +// fmt.Printf("Virt type %s\n", domcfg.Type) +// } // package libvirtxml -- 2.20.1

On Thu, Nov 21, 2019 at 05:18:02PM +0100, Philipp Hahn wrote:
Hello,
as requested here are my proposed changes from <https://www.berrange.com/posts/2017/01/05/announce-new-libvirt-project-go-xml-parser-model/> for libvirt-go-xml/doc.go.
Philipp Hahn (4): doc: Handle conn.LookupDomainByName() error doc: Variable is already defined doc: Add missing newline doc: Make examples a complete program
doc.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-)
Thanks, I've pushed these changes Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (2)
-
Daniel P. Berrangé
-
Philipp Hahn