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

Philipp Hahn posted 6 patches 4 years, 5 months ago
Failed in applying to current master (apply log)
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(-)
[libvirt] [PATCH console-proxy 0/6] libvirt-go-xml and libvirt-console-proxy ?
Posted by Philipp Hahn 4 years, 5 months ago
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


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

Re: [libvirt] [PATCH console-proxy 0/6] libvirt-go-xml and libvirt-console-proxy ?
Posted by Daniel P. Berrangé 4 years, 4 months ago
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 :|

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

[libvirt] [PATCH go-xml 0/4] Update documentation
Posted by Philipp Hahn 4 years, 4 months ago
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


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

Re: [libvirt] [PATCH go-xml 0/4] Update documentation
Posted by Daniel P. Berrangé 4 years, 4 months ago
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 :|

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

[libvirt] [PATCH go-xml 1/4] doc: Handle conn.LookupDomainByName() error
Posted by Philipp Hahn 4 years, 4 months ago
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


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

[libvirt] [PATCH go-xml 2/4] doc: Variable is already defined
Posted by Philipp Hahn 4 years, 4 months ago
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


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

[libvirt] [PATCH go-xml 3/4] doc: Add missing newline
Posted by Philipp Hahn 4 years, 4 months ago
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


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

[libvirt] [PATCH go-xml 4/4] doc: Make examples a complete program
Posted by Philipp Hahn 4 years, 4 months ago
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


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