Cleanup, and fix decoding of TypedParams.

internal/lvgen/gen/main.go

@@ -23,11 +23,14 @@ import (
 )
 
 // TODO: make these an argument
-const lvPath = "../../../libvirt"
 const protoPath = "src/remote/remote_protocol.x"
 
 func main() {
-	fmt.Println("Generating golang bindings for libvirt")
+	lvPath := os.Getenv("LIBVIRT_SOURCE")
+	if lvPath == "" {
+		fmt.Println("set $LIBVIRT_SOURCE to point to the root of the libvirt sources and retry")
+		os.Exit(1)
+	}
 	lvFile := path.Join(lvPath, protoPath)
 	rdr, err := os.Open(lvFile)
 	if err != nil {

internal/lvgen/generate.go

@@ -14,10 +14,6 @@
 
 package lvgen
 
-// The libvirt API is divided into several categories. (Gallia est omnis divisa
-// in partes tres.) The generator will output code for each category in a
-// package underneath the go-libvirt directory.
-
 import (
 	"fmt"
 	"io"
@@ -29,30 +25,6 @@ import (
 	"unicode/utf8"
 )
 
-var keywords = map[string]int{
-	"hyper":    HYPER,
-	"int":      INT,
-	"short":    SHORT,
-	"char":     CHAR,
-	"bool":     BOOL,
-	"case":     CASE,
-	"const":    CONST,
-	"default":  DEFAULT,
-	"double":   DOUBLE,
-	"enum":     ENUM,
-	"float":    FLOAT,
-	"opaque":   OPAQUE,
-	"string":   STRING,
-	"struct":   STRUCT,
-	"switch":   SWITCH,
-	"typedef":  TYPEDEF,
-	"union":    UNION,
-	"unsigned": UNSIGNED,
-	"void":     VOID,
-	"program":  PROGRAM,
-	"version":  VERSION,
-}
-
 // ConstItem stores an const's symbol and value from the parser. This struct is
 // also used for enums.
 type ConstItem struct {
@@ -77,6 +49,8 @@ type Generator struct {
 	Typedefs []Typedef
 	// Unions holds all the discriminated unions.
 	Unions []Union
+	// UnionMap is a map of the unions we find for quick searching.
+	UnionMap map[string]int
 	// Procs holds all the discovered libvirt procedures.
 	Procs []Proc
 }
@@ -104,6 +78,9 @@ var goEquivTypes = map[string]string{
 	// requires us to ditch the typedef that would otherwise be generated.
 	"NonnullString": "string",
 
+	// TODO: Get rid of these. They're only needed because we lose information
+	// that the parser has (the parser knows it has emitted a go type), and then
+	// we capitalize types to make them public.
 	"String":  "string",
 	"Int":     "int",
 	"Uint":    "uint",
@@ -122,7 +99,8 @@ var goEquivTypes = map[string]string{
 }
 
 // These defines are from libvirt-common.h. They should be fetched from there,
-// but for now they're hardcoded here.
+// but for now they're hardcoded here. (These are the discriminant values for
+// TypedParams.)
 var lvTypedParams = map[string]uint32{
 	"VIR_TYPED_PARAM_INT":     1,
 	"VIR_TYPED_PARAM_UINT":    2,
@@ -222,6 +200,7 @@ var CurrentCase *Case
 // generation.
 func Generate(proto io.Reader) error {
 	Gen.StructMap = make(map[string]int)
+	Gen.UnionMap = make(map[string]int)
 	lexer, err := NewLexer(proto)
 	if err != nil {
 		return err
@@ -257,6 +236,8 @@ func Generate(proto io.Reader) error {
 	return err
 }
 
+// genGo is called when the parsing is done; it generates the golang output
+// files using templates.
 func genGo(constFile, procFile io.Writer) error {
 	t, err := template.ParseFiles("constants.tmpl")
 	if err != nil {
@@ -273,18 +254,6 @@ func genGo(constFile, procFile io.Writer) error {
 	if err := t.Execute(procFile, Gen); err != nil {
 		return err
 	}
-	// Now generate the wrappers for libvirt's various public API functions.
-	// for _, c := range Gen.Enums {
-	// This appears to be the name of a libvirt procedure, so sort it into
-	// the right list based on the next part of its name.
-	// segs := camelcase.Split(c.Name)
-	// if len(segs) < 3 || segs[0] != "Proc" {
-	// 	continue
-	// }
-	//category := segs[1]
-
-	//fmt.Println(segs)
-	// }
 
 	return nil
 }
@@ -297,7 +266,7 @@ func constNameTransform(name string) string {
 	decamelize := strings.ContainsRune(name, '_')
 	nn := strings.TrimPrefix(name, "REMOTE_")
 	if decamelize {
-		nn = fromSnakeToCamel(nn, true)
+		nn = fromSnakeToCamel(nn)
 	}
 	nn = fixAbbrevs(nn)
 	return nn
@@ -307,7 +276,7 @@ func identifierTransform(name string) string {
 	decamelize := strings.ContainsRune(name, '_')
 	nn := strings.TrimPrefix(name, "remote_")
 	if decamelize {
-		nn = fromSnakeToCamel(nn, true)
+		nn = fromSnakeToCamel(nn)
 	} else {
 		nn = publicize(nn)
 	}
@@ -337,10 +306,10 @@ func publicize(name string) string {
 // are omitted.
 //
 // ex: "PROC_DOMAIN_GET_METADATA" -> "ProcDomainGetMetadata"
-func fromSnakeToCamel(s string, public bool) string {
+func fromSnakeToCamel(s string) string {
 	buf := make([]rune, 0, len(s))
-	// Start rune may be either upper or lower case.
+	// Start rune will be upper case - we generate all public symbols.
-	hump := public
+	hump := true
 
 	for _, r := range s {
 		if r == '_' {
@@ -496,8 +465,8 @@ func StartStruct(name string) {
 // the now-complete struct definition to the generator's list.
 func AddStruct() {
 	st := *CurrentStruct.pop()
+	Gen.StructMap[st.Name] = len(Gen.Structs)
 	Gen.Structs = append(Gen.Structs, st)
-	Gen.StructMap[st.Name] = len(Gen.Structs) - 1
 }
 
 // StartTypedef is called when the parser finds a typedef.
@@ -516,6 +485,7 @@ func StartUnion(name string) {
 // pointer. We handle unions by declaring an interface for the union type, and
 // adding methods to each of the cases so that they satisfy the interface.
 func AddUnion() {
+	Gen.UnionMap[CurrentUnion.Name] = len(Gen.Unions)
 	Gen.Unions = append(Gen.Unions, *CurrentUnion)
 	CurrentUnion = nil
 }
@@ -532,7 +502,7 @@ func StartCase(dvalue string) {
 	if ix := strings.LastIndexByte(caseName, '_'); ix != -1 {
 		caseName = caseName[ix+1:]
 	}
-	caseName = fromSnakeToCamel(caseName, true)
+	caseName = fromSnakeToCamel(caseName)
 	dv, ok := lvTypedParams[dvalue]
 	if ok {
 		dvalue = strconv.FormatUint(uint64(dv), 10)
@@ -587,11 +557,11 @@ func AddFixedArray(identifier, itype, len string) {
 // Variable-length arrays are prefixed with a 32-bit unsigned length, and may
 // also have a maximum length specified.
 func AddVariableArray(identifier, itype, len string) {
-	// FIXME: This ignores the length restriction, so as of now we can't check
+	// This code ignores the length restriction (array<MAXLEN>), so as of now we
-	// to make sure that we're not exceeding that restriction when we fill in
+	// can't check to make sure that we're not exceeding that restriction when
-	// message buffers. That may not matter, if libvirt's checking is careful
+	// we fill in message buffers. That may not matter, if libvirt's checking is
-	// enough. This could be handled with a map, however.
+	// careful enough.
-	atype := fmt.Sprintf("[]%v", itype)
+	atype := "[]" + itype
 	// Handle strings specially. In the rpcgen definition a string is specified
 	// as a variable-length array, either with or without a max length. We want
 	// these to be go strings, so we'll just remove the array specifier.
@@ -608,3 +578,13 @@ func checkIdentifier(i string) string {
 	}
 	return i
 }
+
+// GetUnion returns the type information for a union. If the provided type name
+// isn't a union, the second return value will be false.
+func (decl *Decl) GetUnion() Union {
+	ix, ok := Gen.UnionMap[decl.Type]
+	if ok {
+		return Gen.Unions[ix]
+	}
+	return Union{}
+}

internal/lvgen/lv-gen.go

@@ -1,4 +1,16 @@
 package lvgen
 
+// This file contains the instructions for regenerating the libvirt bindings.
+// We do this by parsing the remote_protocol.x file included in the libvirt
+// sources. Bindings will be generated if you run `go generate` in this
+// directory.
+
+// Before running `go generate`:
+// 1) Make sure goyacc is installed from golang.org/x/tools (you can use this
+//    command: `go get golang.org/x/tools/...`)
+// 2) Set the environment variable LIBVIRT_SOURCE to point to the top level
+//    directory containing the version of libvirt for which you want to generate
+//    bindings.
+
 //go:generate goyacc sunrpc.y
 //go:generate go run gen/main.go

internal/lvgen/lvlexer.go

@@ -31,6 +31,31 @@ const eof = -1
 // runes.
 var oneRuneTokens = `{}[]<>(),=;:*`
 
+var keywords = map[string]int{
+	"hyper":    HYPER,
+	"int":      INT,
+	"short":    SHORT,
+	"char":     CHAR,
+	"bool":     BOOL,
+	"case":     CASE,
+	"const":    CONST,
+	"default":  DEFAULT,
+	"double":   DOUBLE,
+	"enum":     ENUM,
+	"float":    FLOAT,
+	"opaque":   OPAQUE,
+	"string":   STRING,
+	"struct":   STRUCT,
+	"switch":   SWITCH,
+	"typedef":  TYPEDEF,
+	"union":    UNION,
+	"unsigned": UNSIGNED,
+	"void":     VOID,
+	"program":  PROGRAM,
+	"version":  VERSION,
+}
+
+// item is a lexeme, or what the lexer returns to the parser.
 type item struct {
 	typ          int
 	val          string

internal/lvgen/procedures.tmpl

@@ -8,6 +8,7 @@ package libvirt
 
 import (
 	"bytes"
+	"fmt"
 
 	"github.com/davecgh/go-xdr/xdr2"
 	"github.com/digitalocean/go-libvirt/internal/constants"
@@ -42,10 +43,64 @@ type {{$casetype}} struct {
 	{{.Name}} {{.Type}}
 }
 func New{{$casetype}}(v {{.Type}}) *{{$casetype}} { return &{{$casetype}}{DVal: {{.DiscriminantVal}}, {{.Name}}: v} }
+func Decode{{$casetype}}(dec *xdr.Decoder) (*{{$casetype}}, error) {
+	var v {{.Type}}
+	_, err := dec.Decode(&v)
+	if err != nil {
+		return nil, err
+	}
+	return New{{$casetype}}(v), nil
+}
 func (c *{{$casetype}}) Get() interface{} { return c.{{.Name}} }
 {{end}}
 {{- end}}
 
+// TODO: Generate this.
+func decodeTypedParams(dec *xdr.Decoder) ([]TypedParam, error) {
+	count, _, err := dec.DecodeInt()
+	params := make([]TypedParam, count)
+	if err != nil {
+		return nil, err
+	}
+	for ix := int32(0); ix < count; ix++ {
+		name, _, err := dec.DecodeString()
+		if err != nil {
+			return nil, err
+		}
+		ptype, _, err := dec.DecodeInt()
+		if err != nil {
+			return nil, err
+		}
+		var tpv TypedParamValue
+		switch ptype {
+		case 1:	// TypedParamValueInt
+			tpv, err = DecodeTypedParamValueInt(dec)
+		case 2: // TypedParamValueUint
+			tpv, err = DecodeTypedParamValueUint(dec)
+		case 3: // TypedParamValueLlong
+			tpv, err = DecodeTypedParamValueLlong(dec)
+		case 4: // TypedParamValueUllong
+			tpv, err = DecodeTypedParamValueUllong(dec)
+		case 5: // TypedParamValueDouble
+			tpv, err = DecodeTypedParamValueDouble(dec)
+		case 6: // TypedParamValueBoolean
+			tpv, err = DecodeTypedParamValueBoolean(dec)
+		case 7: // TypedParamValueString
+			tpv, err = DecodeTypedParamValueString(dec)
+		default:
+			err = fmt.Errorf("invalid parameter type %v", ptype)
+		}
+
+		if err != nil {
+			return nil, err
+		}
+
+		params[ix] = TypedParam{name, tpv}
+	}
+
+	return params, nil
+}
+
 // Procedures:
 {{range .Procs}}
 func (l *Libvirt) {{.Name}}({{range $ix, $arg := .Args}}{{if $ix}}, {{end}}{{.Name}} {{.Type}}{{end}}) ({{range .Ret}}r{{.Name}} {{.Type}}, {{end}}err error) {
@@ -72,16 +127,21 @@ func (l *Libvirt) {{.Name}}({{range $ix, $arg := .Args}}{{if $ix}}, {{end}}{{.Na
 		return
 	}
 {{if .RetStruct}}
-	result := {{.RetStruct}}{}
+	// Return value unmarshaling
 	rdr := bytes.NewReader(r.Payload)
 	dec := xdr.NewDecoder(rdr)
-	_, err = dec.Decode(&result)
+{{range .Ret}}	// {{.Name}}: {{.Type}}
+{{if eq .Type "[]TypedParam"}}	// {{.Name}}
+	r{{.Name}}, err = decodeTypedParams(dec)
+	if err != nil {
+		return
+	}
+{{else}}	_, err = dec.Decode(&r{{.Name}})
 	if err != nil {
 		return
 	}
-
-{{range .Ret}}	r{{.Name}} = result.{{.Name}}
 {{end}}{{end}}
+{{end}}
 	return
 }
 {{end}}

libvirt.go

@@ -1398,39 +1398,13 @@ func (l *Libvirt) SetBlockIOTune(dom string, disk string, limits ...BlockLimit)
 		return err
 	}
 
-	// https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetBlockIoTune
+	params := make([]TypedParam, len(limits))
-	payload := struct {
+	for ix, limit := range limits {
-		Domain Domain
-		Disk   string
-		Params []TypedParam
-		Flags  DomainAffectFlags
-	}{
-		Domain: d,
-		Disk:   disk,
-		Flags:  FlagDomainAffectLive,
-	}
-
-	for _, limit := range limits {
 		tpval := NewTypedParamValueUllong(limit.Value)
-		tp := &TypedParam{Field: limit.Name, Value: tpval}
+		params[ix] = TypedParam{Field: limit.Name, Value: tpval}
-		payload.Params = append(payload.Params, *tp)
 	}
 
-	buf, err := encode(&payload)
+	return l.DomainSetBlockIOTune(*d, disk, params, FlagDomainAffectLive)
-	if err != nil {
-		return err
-	}
-	resp, err := l.request(constants.ProcDomainSetBlockIOTune, constants.Program, &buf)
-	if err != nil {
-		return err
-	}
-
-	r := <-resp
-	if r.Status != StatusOK {
-		return decodeError(r.Payload)
-	}
-
-	return nil
 }
 
 // GetBlockIOTune returns a slice containing the current block I/O tunables for
@@ -1441,78 +1415,92 @@ func (l *Libvirt) GetBlockIOTune(dom string, disk string) ([]BlockLimit, error)
 		return nil, err
 	}
 
-	payload := struct {
+	lims, _, err := l.DomainGetBlockIOTune(*d, disk, 32, FlagTypedParamStringOkay)
-		Domain     Domain
-		Disk       []string
-		ParamCount uint32
-		Flags      DomainAffectFlags
-	}{
-		Domain:     d,
-		Disk:       []string{disk},
-		ParamCount: 32,
-		Flags:      FlagTypedParamStringOkay,
-	}
-
-	buf, err := encode(&payload)
 	if err != nil {
 		return nil, err
 	}
-
+	// payload := struct {
-	resp, err := l.request(constants.ProcDomainGetBlockIOTune, constants.Program, &buf)
+	// 	Domain     Domain
-	if err != nil {
+	// 	Disk       []string
-		return nil, err
+	// 	ParamCount uint32
-	}
+	// 	Flags      DomainAffectFlags
-
+	// }{
-	r := <-resp
+	// 	Domain:     d,
-	if r.Status != StatusOK {
+	// 	Disk:       []string{disk},
-		return nil, decodeError(r.Payload)
+	// 	ParamCount: 32,
-	}
+	// 	Flags:      FlagTypedParamStringOkay,
+	// }
+	//
+	// buf, err := encode(&payload)
+	// if err != nil {
+	// 	return nil, err
+	// }
+	//
+	// resp, err := l.request(constants.ProcDomainGetBlockIOTune, constants.Program, &buf)
+	// if err != nil {
+	// 	return nil, err
+	// }
+	//
+	// r := <-resp
+	// if r.Status != StatusOK {
+	// 	return nil, decodeError(r.Payload)
+	// }
 
 	var limits []BlockLimit
-	rdr := bytes.NewReader(r.Payload)
+	// rdr := bytes.NewReader(r.Payload)
-	dec := xdr.NewDecoder(rdr)
+	// dec := xdr.NewDecoder(rdr)
 
 	// find out how many params were returned
-	paramCount, _, err := dec.DecodeInt()
+	// paramCount, _, err := dec.DecodeInt()
-	if err != nil {
+	// if err != nil {
-		return nil, err
+	// 	return nil, err
-	}
+	// }
 
 	// now decode each of the returned TypedParams. To do this we read the field
 	// name and type, then use the type information to decode the value.
-	for param := int32(0); param < paramCount; param++ {
+	for _, lim := range lims {
-		// Get the field name
+		var l BlockLimit
-		name, _, err := dec.DecodeString()
+		name := lim.Field
-		if err != nil {
+		switch lim.Value.Get().(type) {
-			return nil, err
+		case uint64:
-		}
+			l = BlockLimit{Name: name, Value: lim.Value.Get().(uint64)}
-		// ...and the type
-		ptype, _, err := dec.DecodeInt()
-		if err != nil {
-			return nil, err
-		}
-
-		// Now we can read the actual value.
-		switch ptype {
-		case TypeParamULLong:
-			var val uint64
-			_, err = dec.Decode(&val)
-			if err != nil {
-				return nil, err
-			}
-			lim := BlockLimit{name, val}
-			limits = append(limits, lim)
-		case TypeParamString:
-			var val string
-			_, err = dec.Decode(&val)
-			if err != nil {
-				return nil, err
-			}
-			// This routine doesn't currently return strings. As of libvirt 3+,
-			// there's one string here, `group_name`.
 		}
+		limits = append(limits, l)
 	}
 
+	// for param := int32(0); param < paramCount; param++ {
+	// 	// Get the field name
+	// 	name, _, err := dec.DecodeString()
+	// 	if err != nil {
+	// 		return nil, err
+	// 	}
+	// 	// ...and the type
+	// 	ptype, _, err := dec.DecodeInt()
+	// 	if err != nil {
+	// 		return nil, err
+	// 	}
+	//
+	// 	// Now we can read the actual value.
+	// 	switch ptype {
+	// 	case TypeParamULLong:
+	// 		var val uint64
+	// 		_, err = dec.Decode(&val)
+	// 		if err != nil {
+	// 			return nil, err
+	// 		}
+	// 		lim := BlockLimit{name, val}
+	// 		limits = append(limits, lim)
+	// 	case TypeParamString:
+	// 		var val string
+	// 		_, err = dec.Decode(&val)
+	// 		if err != nil {
+	// 			return nil, err
+	// 		}
+	// 		// This routine doesn't currently return strings. As of libvirt 3+,
+	// 		// there's one string here, `group_name`.
+	// 	}
+	// }
+
 	return limits, nil
 }