xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [Xen-devel] [PATCH v3 1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC
@ 2020-01-17 15:57 George Dunlap
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 2/8] go/xenlight: Fix CpuidPoliclyList conversion George Dunlap
                   ` (8 more replies)
  0 siblings, 9 replies; 32+ messages in thread
From: George Dunlap @ 2020-01-17 15:57 UTC (permalink / raw)
  To: xen-devel; +Cc: Nick Rosbrook, George Dunlap

The current fromC array code will do the "magic" casting and
martialling even when num_foo variable is 0.  Go crashes when doing
the cast.

Only do array marshalling if the number of elements is non-zero;
otherwise, leave the target pointer empty (nil for Go slices, NULL for
C arrays).

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
v2:
- Remove toC part of this, which has been folded into Nick's patch
  series.

CC: Nick Rosbrook <rosbrookn@ainfosec.com>
---
 tools/golang/xenlight/gengotypes.py  |   5 +-
 tools/golang/xenlight/helpers.gen.go | 452 +++++++++++++++------------
 2 files changed, 261 insertions(+), 196 deletions(-)

diff --git a/tools/golang/xenlight/gengotypes.py b/tools/golang/xenlight/gengotypes.py
index 27edf66241..ad2c573da9 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -419,7 +419,8 @@ def xenlight_golang_array_from_C(ty = None):
     clenvar    = ty.type.lenvar.name
     golenvar   = xenlight_golang_fmt_name(clenvar,exported=False)
 
-    s += '{} := int(xc.{})\n'.format(golenvar, clenvar)
+    s += 'x.{} = nil\n'.format(goname)
+    s += 'if {} := int(xc.{}); {} > 0 {{\n'.format(golenvar, clenvar, golenvar)
     s += '{} := '.format(cslice)
     s +='(*[1<<28]C.{})(unsafe.Pointer(xc.{}))[:{}:{}]\n'.format(ctypename, cname,
                                                                 golenvar, golenvar)
@@ -433,7 +434,7 @@ def xenlight_golang_array_from_C(ty = None):
         s += 'if err := x.{}[i].fromC(&v); err != nil {{\n'.format(goname)
         s += 'return err }\n'
 
-    s += '}\n'
+    s += '}\n}\n'
 
     return s
 
diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
index b9a7e828a0..889807d928 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -623,12 +623,14 @@ func (x *SchedParams) toC(xc *C.libxl_sched_params) (err error) {
 
 func (x *VcpuSchedParams) fromC(xc *C.libxl_vcpu_sched_params) error {
 	x.Sched = Scheduler(xc.sched)
-	numVcpus := int(xc.num_vcpus)
-	cVcpus := (*[1 << 28]C.libxl_sched_params)(unsafe.Pointer(xc.vcpus))[:numVcpus:numVcpus]
-	x.Vcpus = make([]SchedParams, numVcpus)
-	for i, v := range cVcpus {
-		if err := x.Vcpus[i].fromC(&v); err != nil {
-			return err
+	x.Vcpus = nil
+	if numVcpus := int(xc.num_vcpus); numVcpus > 0 {
+		cVcpus := (*[1 << 28]C.libxl_sched_params)(unsafe.Pointer(xc.vcpus))[:numVcpus:numVcpus]
+		x.Vcpus = make([]SchedParams, numVcpus)
+		for i, v := range cVcpus {
+			if err := x.Vcpus[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
 
@@ -691,11 +693,13 @@ func (x *DomainSchedParams) toC(xc *C.libxl_domain_sched_params) (err error) {
 
 func (x *VnodeInfo) fromC(xc *C.libxl_vnode_info) error {
 	x.Memkb = uint64(xc.memkb)
-	numDistances := int(xc.num_distances)
-	cDistances := (*[1 << 28]C.uint32_t)(unsafe.Pointer(xc.distances))[:numDistances:numDistances]
-	x.Distances = make([]uint32, numDistances)
-	for i, v := range cDistances {
-		x.Distances[i] = uint32(v)
+	x.Distances = nil
+	if numDistances := int(xc.num_distances); numDistances > 0 {
+		cDistances := (*[1 << 28]C.uint32_t)(unsafe.Pointer(xc.distances))[:numDistances:numDistances]
+		x.Distances = make([]uint32, numDistances)
+		for i, v := range cDistances {
+			x.Distances[i] = uint32(v)
+		}
 	}
 	x.Pnode = uint32(xc.pnode)
 	if err := x.Vcpus.fromC(&xc.vcpus); err != nil {
@@ -760,20 +764,24 @@ func (x *DomainBuildInfo) fromC(xc *C.libxl_domain_build_info) error {
 	if err := x.Nodemap.fromC(&xc.nodemap); err != nil {
 		return err
 	}
-	numVcpuHardAffinity := int(xc.num_vcpu_hard_affinity)
-	cVcpuHardAffinity := (*[1 << 28]C.libxl_bitmap)(unsafe.Pointer(xc.vcpu_hard_affinity))[:numVcpuHardAffinity:numVcpuHardAffinity]
-	x.VcpuHardAffinity = make([]Bitmap, numVcpuHardAffinity)
-	for i, v := range cVcpuHardAffinity {
-		if err := x.VcpuHardAffinity[i].fromC(&v); err != nil {
-			return err
+	x.VcpuHardAffinity = nil
+	if numVcpuHardAffinity := int(xc.num_vcpu_hard_affinity); numVcpuHardAffinity > 0 {
+		cVcpuHardAffinity := (*[1 << 28]C.libxl_bitmap)(unsafe.Pointer(xc.vcpu_hard_affinity))[:numVcpuHardAffinity:numVcpuHardAffinity]
+		x.VcpuHardAffinity = make([]Bitmap, numVcpuHardAffinity)
+		for i, v := range cVcpuHardAffinity {
+			if err := x.VcpuHardAffinity[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
-	numVcpuSoftAffinity := int(xc.num_vcpu_soft_affinity)
-	cVcpuSoftAffinity := (*[1 << 28]C.libxl_bitmap)(unsafe.Pointer(xc.vcpu_soft_affinity))[:numVcpuSoftAffinity:numVcpuSoftAffinity]
-	x.VcpuSoftAffinity = make([]Bitmap, numVcpuSoftAffinity)
-	for i, v := range cVcpuSoftAffinity {
-		if err := x.VcpuSoftAffinity[i].fromC(&v); err != nil {
-			return err
+	x.VcpuSoftAffinity = nil
+	if numVcpuSoftAffinity := int(xc.num_vcpu_soft_affinity); numVcpuSoftAffinity > 0 {
+		cVcpuSoftAffinity := (*[1 << 28]C.libxl_bitmap)(unsafe.Pointer(xc.vcpu_soft_affinity))[:numVcpuSoftAffinity:numVcpuSoftAffinity]
+		x.VcpuSoftAffinity = make([]Bitmap, numVcpuSoftAffinity)
+		for i, v := range cVcpuSoftAffinity {
+			if err := x.VcpuSoftAffinity[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
 	if err := x.NumaPlacement.fromC(&xc.numa_placement); err != nil {
@@ -798,12 +806,14 @@ func (x *DomainBuildInfo) fromC(xc *C.libxl_domain_build_info) error {
 		return err
 	}
 	x.BlkdevStart = C.GoString(xc.blkdev_start)
-	numVnumaNodes := int(xc.num_vnuma_nodes)
-	cVnumaNodes := (*[1 << 28]C.libxl_vnode_info)(unsafe.Pointer(xc.vnuma_nodes))[:numVnumaNodes:numVnumaNodes]
-	x.VnumaNodes = make([]VnodeInfo, numVnumaNodes)
-	for i, v := range cVnumaNodes {
-		if err := x.VnumaNodes[i].fromC(&v); err != nil {
-			return err
+	x.VnumaNodes = nil
+	if numVnumaNodes := int(xc.num_vnuma_nodes); numVnumaNodes > 0 {
+		cVnumaNodes := (*[1 << 28]C.libxl_vnode_info)(unsafe.Pointer(xc.vnuma_nodes))[:numVnumaNodes:numVnumaNodes]
+		x.VnumaNodes = make([]VnodeInfo, numVnumaNodes)
+		for i, v := range cVnumaNodes {
+			if err := x.VnumaNodes[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
 	x.MaxGrantFrames = uint32(xc.max_grant_frames)
@@ -828,26 +838,32 @@ func (x *DomainBuildInfo) fromC(xc *C.libxl_domain_build_info) error {
 	if err := x.SchedParams.fromC(&xc.sched_params); err != nil {
 		return err
 	}
-	numIoports := int(xc.num_ioports)
-	cIoports := (*[1 << 28]C.libxl_ioport_range)(unsafe.Pointer(xc.ioports))[:numIoports:numIoports]
-	x.Ioports = make([]IoportRange, numIoports)
-	for i, v := range cIoports {
-		if err := x.Ioports[i].fromC(&v); err != nil {
-			return err
+	x.Ioports = nil
+	if numIoports := int(xc.num_ioports); numIoports > 0 {
+		cIoports := (*[1 << 28]C.libxl_ioport_range)(unsafe.Pointer(xc.ioports))[:numIoports:numIoports]
+		x.Ioports = make([]IoportRange, numIoports)
+		for i, v := range cIoports {
+			if err := x.Ioports[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
-	numIrqs := int(xc.num_irqs)
-	cIrqs := (*[1 << 28]C.uint32_t)(unsafe.Pointer(xc.irqs))[:numIrqs:numIrqs]
-	x.Irqs = make([]uint32, numIrqs)
-	for i, v := range cIrqs {
-		x.Irqs[i] = uint32(v)
+	x.Irqs = nil
+	if numIrqs := int(xc.num_irqs); numIrqs > 0 {
+		cIrqs := (*[1 << 28]C.uint32_t)(unsafe.Pointer(xc.irqs))[:numIrqs:numIrqs]
+		x.Irqs = make([]uint32, numIrqs)
+		for i, v := range cIrqs {
+			x.Irqs[i] = uint32(v)
+		}
 	}
-	numIomem := int(xc.num_iomem)
-	cIomem := (*[1 << 28]C.libxl_iomem_range)(unsafe.Pointer(xc.iomem))[:numIomem:numIomem]
-	x.Iomem = make([]IomemRange, numIomem)
-	for i, v := range cIomem {
-		if err := x.Iomem[i].fromC(&v); err != nil {
-			return err
+	x.Iomem = nil
+	if numIomem := int(xc.num_iomem); numIomem > 0 {
+		cIomem := (*[1 << 28]C.libxl_iomem_range)(unsafe.Pointer(xc.iomem))[:numIomem:numIomem]
+		x.Iomem = make([]IomemRange, numIomem)
+		for i, v := range cIomem {
+			if err := x.Iomem[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
 	if err := x.ClaimMode.fromC(&xc.claim_mode); err != nil {
@@ -878,18 +894,18 @@ func (x *DomainBuildInfo) fromC(xc *C.libxl_domain_build_info) error {
 	x.Tee = TeeType(xc.tee)
 	x.Type = DomainType(xc._type)
 	switch x.Type {
-	case DomainTypePv:
-		var typePv DomainBuildInfoTypeUnionPv
-		if err := typePv.fromC(xc); err != nil {
-			return err
-		}
-		x.TypeUnion = typePv
 	case DomainTypeHvm:
 		var typeHvm DomainBuildInfoTypeUnionHvm
 		if err := typeHvm.fromC(xc); err != nil {
 			return err
 		}
 		x.TypeUnion = typeHvm
+	case DomainTypePv:
+		var typePv DomainBuildInfoTypeUnionPv
+		if err := typePv.fromC(xc); err != nil {
+			return err
+		}
+		x.TypeUnion = typePv
 	case DomainTypePvh:
 		var typePvh DomainBuildInfoTypeUnionPvh
 		if err := typePvh.fromC(xc); err != nil {
@@ -2187,12 +2203,14 @@ func (x *DeviceVdispl) fromC(xc *C.libxl_device_vdispl) error {
 	x.BackendDomname = C.GoString(xc.backend_domname)
 	x.Devid = Devid(xc.devid)
 	x.BeAlloc = bool(xc.be_alloc)
-	numConnectors := int(xc.num_connectors)
-	cConnectors := (*[1 << 28]C.libxl_connector_param)(unsafe.Pointer(xc.connectors))[:numConnectors:numConnectors]
-	x.Connectors = make([]ConnectorParam, numConnectors)
-	for i, v := range cConnectors {
-		if err := x.Connectors[i].fromC(&v); err != nil {
-			return err
+	x.Connectors = nil
+	if numConnectors := int(xc.num_connectors); numConnectors > 0 {
+		cConnectors := (*[1 << 28]C.libxl_connector_param)(unsafe.Pointer(xc.connectors))[:numConnectors:numConnectors]
+		x.Connectors = make([]ConnectorParam, numConnectors)
+		for i, v := range cConnectors {
+			if err := x.Connectors[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
 
@@ -2227,17 +2245,21 @@ func (x *DeviceVdispl) toC(xc *C.libxl_device_vdispl) (err error) {
 }
 
 func (x *VsndParams) fromC(xc *C.libxl_vsnd_params) error {
-	numSampleRates := int(xc.num_sample_rates)
-	cSampleRates := (*[1 << 28]C.uint32_t)(unsafe.Pointer(xc.sample_rates))[:numSampleRates:numSampleRates]
-	x.SampleRates = make([]uint32, numSampleRates)
-	for i, v := range cSampleRates {
-		x.SampleRates[i] = uint32(v)
-	}
-	numSampleFormats := int(xc.num_sample_formats)
-	cSampleFormats := (*[1 << 28]C.libxl_vsnd_pcm_format)(unsafe.Pointer(xc.sample_formats))[:numSampleFormats:numSampleFormats]
-	x.SampleFormats = make([]VsndPcmFormat, numSampleFormats)
-	for i, v := range cSampleFormats {
-		x.SampleFormats[i] = VsndPcmFormat(v)
+	x.SampleRates = nil
+	if numSampleRates := int(xc.num_sample_rates); numSampleRates > 0 {
+		cSampleRates := (*[1 << 28]C.uint32_t)(unsafe.Pointer(xc.sample_rates))[:numSampleRates:numSampleRates]
+		x.SampleRates = make([]uint32, numSampleRates)
+		for i, v := range cSampleRates {
+			x.SampleRates[i] = uint32(v)
+		}
+	}
+	x.SampleFormats = nil
+	if numSampleFormats := int(xc.num_sample_formats); numSampleFormats > 0 {
+		cSampleFormats := (*[1 << 28]C.libxl_vsnd_pcm_format)(unsafe.Pointer(xc.sample_formats))[:numSampleFormats:numSampleFormats]
+		x.SampleFormats = make([]VsndPcmFormat, numSampleFormats)
+		for i, v := range cSampleFormats {
+			x.SampleFormats[i] = VsndPcmFormat(v)
+		}
 	}
 	x.ChannelsMin = uint32(xc.channels_min)
 	x.ChannelsMax = uint32(xc.channels_max)
@@ -2309,12 +2331,14 @@ func (x *VsndPcm) fromC(xc *C.libxl_vsnd_pcm) error {
 	if err := x.Params.fromC(&xc.params); err != nil {
 		return err
 	}
-	numVsndStreams := int(xc.num_vsnd_streams)
-	cStreams := (*[1 << 28]C.libxl_vsnd_stream)(unsafe.Pointer(xc.streams))[:numVsndStreams:numVsndStreams]
-	x.Streams = make([]VsndStream, numVsndStreams)
-	for i, v := range cStreams {
-		if err := x.Streams[i].fromC(&v); err != nil {
-			return err
+	x.Streams = nil
+	if numVsndStreams := int(xc.num_vsnd_streams); numVsndStreams > 0 {
+		cStreams := (*[1 << 28]C.libxl_vsnd_stream)(unsafe.Pointer(xc.streams))[:numVsndStreams:numVsndStreams]
+		x.Streams = make([]VsndStream, numVsndStreams)
+		for i, v := range cStreams {
+			if err := x.Streams[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
 
@@ -2357,12 +2381,14 @@ func (x *DeviceVsnd) fromC(xc *C.libxl_device_vsnd) error {
 	if err := x.Params.fromC(&xc.params); err != nil {
 		return err
 	}
-	numVsndPcms := int(xc.num_vsnd_pcms)
-	cPcms := (*[1 << 28]C.libxl_vsnd_pcm)(unsafe.Pointer(xc.pcms))[:numVsndPcms:numVsndPcms]
-	x.Pcms = make([]VsndPcm, numVsndPcms)
-	for i, v := range cPcms {
-		if err := x.Pcms[i].fromC(&v); err != nil {
-			return err
+	x.Pcms = nil
+	if numVsndPcms := int(xc.num_vsnd_pcms); numVsndPcms > 0 {
+		cPcms := (*[1 << 28]C.libxl_vsnd_pcm)(unsafe.Pointer(xc.pcms))[:numVsndPcms:numVsndPcms]
+		x.Pcms = make([]VsndPcm, numVsndPcms)
+		for i, v := range cPcms {
+			if err := x.Pcms[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
 
@@ -2411,124 +2437,154 @@ func (x *DomainConfig) fromC(xc *C.libxl_domain_config) error {
 	if err := x.BInfo.fromC(&xc.b_info); err != nil {
 		return err
 	}
-	numDisks := int(xc.num_disks)
-	cDisks := (*[1 << 28]C.libxl_device_disk)(unsafe.Pointer(xc.disks))[:numDisks:numDisks]
-	x.Disks = make([]DeviceDisk, numDisks)
-	for i, v := range cDisks {
-		if err := x.Disks[i].fromC(&v); err != nil {
-			return err
+	x.Disks = nil
+	if numDisks := int(xc.num_disks); numDisks > 0 {
+		cDisks := (*[1 << 28]C.libxl_device_disk)(unsafe.Pointer(xc.disks))[:numDisks:numDisks]
+		x.Disks = make([]DeviceDisk, numDisks)
+		for i, v := range cDisks {
+			if err := x.Disks[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
-	numNics := int(xc.num_nics)
-	cNics := (*[1 << 28]C.libxl_device_nic)(unsafe.Pointer(xc.nics))[:numNics:numNics]
-	x.Nics = make([]DeviceNic, numNics)
-	for i, v := range cNics {
-		if err := x.Nics[i].fromC(&v); err != nil {
-			return err
+	x.Nics = nil
+	if numNics := int(xc.num_nics); numNics > 0 {
+		cNics := (*[1 << 28]C.libxl_device_nic)(unsafe.Pointer(xc.nics))[:numNics:numNics]
+		x.Nics = make([]DeviceNic, numNics)
+		for i, v := range cNics {
+			if err := x.Nics[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
-	numPcidevs := int(xc.num_pcidevs)
-	cPcidevs := (*[1 << 28]C.libxl_device_pci)(unsafe.Pointer(xc.pcidevs))[:numPcidevs:numPcidevs]
-	x.Pcidevs = make([]DevicePci, numPcidevs)
-	for i, v := range cPcidevs {
-		if err := x.Pcidevs[i].fromC(&v); err != nil {
-			return err
+	x.Pcidevs = nil
+	if numPcidevs := int(xc.num_pcidevs); numPcidevs > 0 {
+		cPcidevs := (*[1 << 28]C.libxl_device_pci)(unsafe.Pointer(xc.pcidevs))[:numPcidevs:numPcidevs]
+		x.Pcidevs = make([]DevicePci, numPcidevs)
+		for i, v := range cPcidevs {
+			if err := x.Pcidevs[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
-	numRdms := int(xc.num_rdms)
-	cRdms := (*[1 << 28]C.libxl_device_rdm)(unsafe.Pointer(xc.rdms))[:numRdms:numRdms]
-	x.Rdms = make([]DeviceRdm, numRdms)
-	for i, v := range cRdms {
-		if err := x.Rdms[i].fromC(&v); err != nil {
-			return err
+	x.Rdms = nil
+	if numRdms := int(xc.num_rdms); numRdms > 0 {
+		cRdms := (*[1 << 28]C.libxl_device_rdm)(unsafe.Pointer(xc.rdms))[:numRdms:numRdms]
+		x.Rdms = make([]DeviceRdm, numRdms)
+		for i, v := range cRdms {
+			if err := x.Rdms[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
-	numDtdevs := int(xc.num_dtdevs)
-	cDtdevs := (*[1 << 28]C.libxl_device_dtdev)(unsafe.Pointer(xc.dtdevs))[:numDtdevs:numDtdevs]
-	x.Dtdevs = make([]DeviceDtdev, numDtdevs)
-	for i, v := range cDtdevs {
-		if err := x.Dtdevs[i].fromC(&v); err != nil {
-			return err
+	x.Dtdevs = nil
+	if numDtdevs := int(xc.num_dtdevs); numDtdevs > 0 {
+		cDtdevs := (*[1 << 28]C.libxl_device_dtdev)(unsafe.Pointer(xc.dtdevs))[:numDtdevs:numDtdevs]
+		x.Dtdevs = make([]DeviceDtdev, numDtdevs)
+		for i, v := range cDtdevs {
+			if err := x.Dtdevs[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
-	numVfbs := int(xc.num_vfbs)
-	cVfbs := (*[1 << 28]C.libxl_device_vfb)(unsafe.Pointer(xc.vfbs))[:numVfbs:numVfbs]
-	x.Vfbs = make([]DeviceVfb, numVfbs)
-	for i, v := range cVfbs {
-		if err := x.Vfbs[i].fromC(&v); err != nil {
-			return err
+	x.Vfbs = nil
+	if numVfbs := int(xc.num_vfbs); numVfbs > 0 {
+		cVfbs := (*[1 << 28]C.libxl_device_vfb)(unsafe.Pointer(xc.vfbs))[:numVfbs:numVfbs]
+		x.Vfbs = make([]DeviceVfb, numVfbs)
+		for i, v := range cVfbs {
+			if err := x.Vfbs[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
-	numVkbs := int(xc.num_vkbs)
-	cVkbs := (*[1 << 28]C.libxl_device_vkb)(unsafe.Pointer(xc.vkbs))[:numVkbs:numVkbs]
-	x.Vkbs = make([]DeviceVkb, numVkbs)
-	for i, v := range cVkbs {
-		if err := x.Vkbs[i].fromC(&v); err != nil {
-			return err
+	x.Vkbs = nil
+	if numVkbs := int(xc.num_vkbs); numVkbs > 0 {
+		cVkbs := (*[1 << 28]C.libxl_device_vkb)(unsafe.Pointer(xc.vkbs))[:numVkbs:numVkbs]
+		x.Vkbs = make([]DeviceVkb, numVkbs)
+		for i, v := range cVkbs {
+			if err := x.Vkbs[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
-	numVtpms := int(xc.num_vtpms)
-	cVtpms := (*[1 << 28]C.libxl_device_vtpm)(unsafe.Pointer(xc.vtpms))[:numVtpms:numVtpms]
-	x.Vtpms = make([]DeviceVtpm, numVtpms)
-	for i, v := range cVtpms {
-		if err := x.Vtpms[i].fromC(&v); err != nil {
-			return err
+	x.Vtpms = nil
+	if numVtpms := int(xc.num_vtpms); numVtpms > 0 {
+		cVtpms := (*[1 << 28]C.libxl_device_vtpm)(unsafe.Pointer(xc.vtpms))[:numVtpms:numVtpms]
+		x.Vtpms = make([]DeviceVtpm, numVtpms)
+		for i, v := range cVtpms {
+			if err := x.Vtpms[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
-	numP9S := int(xc.num_p9s)
-	cP9S := (*[1 << 28]C.libxl_device_p9)(unsafe.Pointer(xc.p9s))[:numP9S:numP9S]
-	x.P9S = make([]DeviceP9, numP9S)
-	for i, v := range cP9S {
-		if err := x.P9S[i].fromC(&v); err != nil {
-			return err
+	x.P9S = nil
+	if numP9S := int(xc.num_p9s); numP9S > 0 {
+		cP9S := (*[1 << 28]C.libxl_device_p9)(unsafe.Pointer(xc.p9s))[:numP9S:numP9S]
+		x.P9S = make([]DeviceP9, numP9S)
+		for i, v := range cP9S {
+			if err := x.P9S[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
-	numPvcallsifs := int(xc.num_pvcallsifs)
-	cPvcallsifs := (*[1 << 28]C.libxl_device_pvcallsif)(unsafe.Pointer(xc.pvcallsifs))[:numPvcallsifs:numPvcallsifs]
-	x.Pvcallsifs = make([]DevicePvcallsif, numPvcallsifs)
-	for i, v := range cPvcallsifs {
-		if err := x.Pvcallsifs[i].fromC(&v); err != nil {
-			return err
+	x.Pvcallsifs = nil
+	if numPvcallsifs := int(xc.num_pvcallsifs); numPvcallsifs > 0 {
+		cPvcallsifs := (*[1 << 28]C.libxl_device_pvcallsif)(unsafe.Pointer(xc.pvcallsifs))[:numPvcallsifs:numPvcallsifs]
+		x.Pvcallsifs = make([]DevicePvcallsif, numPvcallsifs)
+		for i, v := range cPvcallsifs {
+			if err := x.Pvcallsifs[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
-	numVdispls := int(xc.num_vdispls)
-	cVdispls := (*[1 << 28]C.libxl_device_vdispl)(unsafe.Pointer(xc.vdispls))[:numVdispls:numVdispls]
-	x.Vdispls = make([]DeviceVdispl, numVdispls)
-	for i, v := range cVdispls {
-		if err := x.Vdispls[i].fromC(&v); err != nil {
-			return err
+	x.Vdispls = nil
+	if numVdispls := int(xc.num_vdispls); numVdispls > 0 {
+		cVdispls := (*[1 << 28]C.libxl_device_vdispl)(unsafe.Pointer(xc.vdispls))[:numVdispls:numVdispls]
+		x.Vdispls = make([]DeviceVdispl, numVdispls)
+		for i, v := range cVdispls {
+			if err := x.Vdispls[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
-	numVsnds := int(xc.num_vsnds)
-	cVsnds := (*[1 << 28]C.libxl_device_vsnd)(unsafe.Pointer(xc.vsnds))[:numVsnds:numVsnds]
-	x.Vsnds = make([]DeviceVsnd, numVsnds)
-	for i, v := range cVsnds {
-		if err := x.Vsnds[i].fromC(&v); err != nil {
-			return err
+	x.Vsnds = nil
+	if numVsnds := int(xc.num_vsnds); numVsnds > 0 {
+		cVsnds := (*[1 << 28]C.libxl_device_vsnd)(unsafe.Pointer(xc.vsnds))[:numVsnds:numVsnds]
+		x.Vsnds = make([]DeviceVsnd, numVsnds)
+		for i, v := range cVsnds {
+			if err := x.Vsnds[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
-	numChannels := int(xc.num_channels)
-	cChannels := (*[1 << 28]C.libxl_device_channel)(unsafe.Pointer(xc.channels))[:numChannels:numChannels]
-	x.Channels = make([]DeviceChannel, numChannels)
-	for i, v := range cChannels {
-		if err := x.Channels[i].fromC(&v); err != nil {
-			return err
+	x.Channels = nil
+	if numChannels := int(xc.num_channels); numChannels > 0 {
+		cChannels := (*[1 << 28]C.libxl_device_channel)(unsafe.Pointer(xc.channels))[:numChannels:numChannels]
+		x.Channels = make([]DeviceChannel, numChannels)
+		for i, v := range cChannels {
+			if err := x.Channels[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
-	numUsbctrls := int(xc.num_usbctrls)
-	cUsbctrls := (*[1 << 28]C.libxl_device_usbctrl)(unsafe.Pointer(xc.usbctrls))[:numUsbctrls:numUsbctrls]
-	x.Usbctrls = make([]DeviceUsbctrl, numUsbctrls)
-	for i, v := range cUsbctrls {
-		if err := x.Usbctrls[i].fromC(&v); err != nil {
-			return err
+	x.Usbctrls = nil
+	if numUsbctrls := int(xc.num_usbctrls); numUsbctrls > 0 {
+		cUsbctrls := (*[1 << 28]C.libxl_device_usbctrl)(unsafe.Pointer(xc.usbctrls))[:numUsbctrls:numUsbctrls]
+		x.Usbctrls = make([]DeviceUsbctrl, numUsbctrls)
+		for i, v := range cUsbctrls {
+			if err := x.Usbctrls[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
-	numUsbdevs := int(xc.num_usbdevs)
-	cUsbdevs := (*[1 << 28]C.libxl_device_usbdev)(unsafe.Pointer(xc.usbdevs))[:numUsbdevs:numUsbdevs]
-	x.Usbdevs = make([]DeviceUsbdev, numUsbdevs)
-	for i, v := range cUsbdevs {
-		if err := x.Usbdevs[i].fromC(&v); err != nil {
-			return err
+	x.Usbdevs = nil
+	if numUsbdevs := int(xc.num_usbdevs); numUsbdevs > 0 {
+		cUsbdevs := (*[1 << 28]C.libxl_device_usbdev)(unsafe.Pointer(xc.usbdevs))[:numUsbdevs:numUsbdevs]
+		x.Usbdevs = make([]DeviceUsbdev, numUsbdevs)
+		for i, v := range cUsbdevs {
+			if err := x.Usbdevs[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
 	x.OnPoweroff = ActionOnShutdown(xc.on_poweroff)
@@ -3012,12 +3068,14 @@ func (x *Vdisplinfo) fromC(xc *C.libxl_vdisplinfo) error {
 	x.Devid = Devid(xc.devid)
 	x.State = int(xc.state)
 	x.BeAlloc = bool(xc.be_alloc)
-	numConnectors := int(xc.num_connectors)
-	cConnectors := (*[1 << 28]C.libxl_connectorinfo)(unsafe.Pointer(xc.connectors))[:numConnectors:numConnectors]
-	x.Connectors = make([]Connectorinfo, numConnectors)
-	for i, v := range cConnectors {
-		if err := x.Connectors[i].fromC(&v); err != nil {
-			return err
+	x.Connectors = nil
+	if numConnectors := int(xc.num_connectors); numConnectors > 0 {
+		cConnectors := (*[1 << 28]C.libxl_connectorinfo)(unsafe.Pointer(xc.connectors))[:numConnectors:numConnectors]
+		x.Connectors = make([]Connectorinfo, numConnectors)
+		for i, v := range cConnectors {
+			if err := x.Connectors[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
 
@@ -3077,12 +3135,14 @@ func (x *Streaminfo) toC(xc *C.libxl_streaminfo) (err error) {
 }
 
 func (x *Pcminfo) fromC(xc *C.libxl_pcminfo) error {
-	numVsndStreams := int(xc.num_vsnd_streams)
-	cStreams := (*[1 << 28]C.libxl_streaminfo)(unsafe.Pointer(xc.streams))[:numVsndStreams:numVsndStreams]
-	x.Streams = make([]Streaminfo, numVsndStreams)
-	for i, v := range cStreams {
-		if err := x.Streams[i].fromC(&v); err != nil {
-			return err
+	x.Streams = nil
+	if numVsndStreams := int(xc.num_vsnd_streams); numVsndStreams > 0 {
+		cStreams := (*[1 << 28]C.libxl_streaminfo)(unsafe.Pointer(xc.streams))[:numVsndStreams:numVsndStreams]
+		x.Streams = make([]Streaminfo, numVsndStreams)
+		for i, v := range cStreams {
+			if err := x.Streams[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
 
@@ -3117,12 +3177,14 @@ func (x *Vsndinfo) fromC(xc *C.libxl_vsndinfo) error {
 	x.FrontendId = uint32(xc.frontend_id)
 	x.Devid = Devid(xc.devid)
 	x.State = int(xc.state)
-	numVsndPcms := int(xc.num_vsnd_pcms)
-	cPcms := (*[1 << 28]C.libxl_pcminfo)(unsafe.Pointer(xc.pcms))[:numVsndPcms:numVsndPcms]
-	x.Pcms = make([]Pcminfo, numVsndPcms)
-	for i, v := range cPcms {
-		if err := x.Pcms[i].fromC(&v); err != nil {
-			return err
+	x.Pcms = nil
+	if numVsndPcms := int(xc.num_vsnd_pcms); numVsndPcms > 0 {
+		cPcms := (*[1 << 28]C.libxl_pcminfo)(unsafe.Pointer(xc.pcms))[:numVsndPcms:numVsndPcms]
+		x.Pcms = make([]Pcminfo, numVsndPcms)
+		for i, v := range cPcms {
+			if err := x.Pcms[i].fromC(&v); err != nil {
+				return err
+			}
 		}
 	}
 
@@ -3199,11 +3261,13 @@ func (x *Vkbinfo) toC(xc *C.libxl_vkbinfo) (err error) {
 func (x *Numainfo) fromC(xc *C.libxl_numainfo) error {
 	x.Size = uint64(xc.size)
 	x.Free = uint64(xc.free)
-	numDists := int(xc.num_dists)
-	cDists := (*[1 << 28]C.uint32_t)(unsafe.Pointer(xc.dists))[:numDists:numDists]
-	x.Dists = make([]uint32, numDists)
-	for i, v := range cDists {
-		x.Dists[i] = uint32(v)
+	x.Dists = nil
+	if numDists := int(xc.num_dists); numDists > 0 {
+		cDists := (*[1 << 28]C.uint32_t)(unsafe.Pointer(xc.dists))[:numDists:numDists]
+		x.Dists = make([]uint32, numDists)
+		for i, v := range cDists {
+			x.Dists[i] = uint32(v)
+		}
 	}
 
 	return nil
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Xen-devel] [PATCH v3 2/8] go/xenlight: Fix CpuidPoliclyList conversion
  2020-01-17 15:57 [Xen-devel] [PATCH v3 1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC George Dunlap
@ 2020-01-17 15:57 ` George Dunlap
  2020-01-20 23:30   ` Nick Rosbrook
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 3/8] go/xenlight: More informative error messages George Dunlap
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: George Dunlap @ 2020-01-17 15:57 UTC (permalink / raw)
  To: xen-devel; +Cc: Nick Rosbrook, George Dunlap

Empty Go strings should be converted to `nil` libxl_cpuid_policy_list;
otherwise libxl_cpuid_parse_config gets confused.

Also, libxl_cpuid_policy_list returns a weird error, not a "normal"
libxl error; if it returns one of these non-standard errors, convert
it to ErrorInval.

Finally, make the fromC() method take a pointer, and set the value of
CpuidPolicyList such that it will generate a valid CpuidPolicyList in
response.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
v2:
- Port over toC() function signature change
- Have fromC set the string to ""

CC: Nick Rosbrook <rosbrookn@ainfosec.com>
---
 tools/golang/xenlight/xenlight.go | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index b1587b964f..1299981713 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -306,9 +306,14 @@ func (el *EvLink) toC(cel *C.libxl_ev_link) (err error) { return }
 // empty when it is returned from libxl.
 type CpuidPolicyList string
 
-func (cpl CpuidPolicyList) fromC(ccpl *C.libxl_cpuid_policy_list) error { return nil }
+func (cpl *CpuidPolicyList) fromC(ccpl *C.libxl_cpuid_policy_list) error { *cpl = ""; return nil }
 
 func (cpl CpuidPolicyList) toC(ccpl *C.libxl_cpuid_policy_list) error {
+	if cpl == "" {
+		*ccpl = nil
+		return nil
+	}
+
 	s := C.CString(string(cpl))
 	defer C.free(unsafe.Pointer(s))
 
@@ -316,7 +321,8 @@ func (cpl CpuidPolicyList) toC(ccpl *C.libxl_cpuid_policy_list) error {
 	if ret != 0 {
 		C.libxl_cpuid_dispose(ccpl)
 
-		return Error(-ret)
+		// libxl_cpuid_parse_config doesn't return a normal libxl error.
+		return ErrorInval
 	}
 
 	return nil
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Xen-devel] [PATCH v3 3/8] go/xenlight: More informative error messages
  2020-01-17 15:57 [Xen-devel] [PATCH v3 1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC George Dunlap
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 2/8] go/xenlight: Fix CpuidPoliclyList conversion George Dunlap
@ 2020-01-17 15:57 ` George Dunlap
  2020-01-20 23:32   ` Nick Rosbrook
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 4/8] golang/xenlight: Errors are negative George Dunlap
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: George Dunlap @ 2020-01-17 15:57 UTC (permalink / raw)
  To: xen-devel; +Cc: Nick Rosbrook, George Dunlap

If an error is encountered deep in a complicated data structure, it's
often difficult to tell where the error actually is.  Make the error
message from the generated toC() and fromC() structures more
informative by tagging which field being converted encountered the
error.  This will have the effect of giving a "stack trace" of the
failure inside a nested data structure.

NB that my version of python insists on reordering a couple of switch
statements for some reason; In other patches I've reverted those
changes, but in this case it's more difficult because they interact
with actual code changes.  I'll leave this here for now, as we're
going to remove helpers.gen.go from being tracked by git at some point
in the near future anyway.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
v2:
- Keep error messages lower case
- Actually implement .toC changes

CC: Nick Rosbrook <rosbrookn@ainfosec.com>
---
 tools/golang/xenlight/gengotypes.py  |  10 +-
 tools/golang/xenlight/helpers.gen.go | 530 +++++++++++++--------------
 2 files changed, 270 insertions(+), 270 deletions(-)

diff --git a/tools/golang/xenlight/gengotypes.py b/tools/golang/xenlight/gengotypes.py
index ad2c573da9..17b0ca00bd 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -314,7 +314,7 @@ def xenlight_golang_convert_from_C(ty = None, outer_name = None, cvarname = None
         # If the type is not castable, we need to call its fromC
         # function.
         s += 'if err := x.{}.fromC(&{}.{});'.format(goname,cvarname,cname)
-        s += 'err != nil {\n return err \n}\n'
+        s += 'err != nil {{\nreturn fmt.Errorf("converting field {}: %v", err) \n}}\n'.format(goname)
 
     elif gotypename == 'string':
         # Use the cgo helper for converting C strings.
@@ -389,7 +389,7 @@ def xenlight_golang_union_from_C(ty = None, union_name = '', struct_name = ''):
 
         s += 'var {} {}\n'.format(goname, gotype)
         s += 'if err := {}.fromC(xc);'.format(goname)
-        s += 'err != nil {\n return err \n}\n'
+        s += 'err != nil {{\n return fmt.Errorf("converting field {}: %v", err) \n}}\n'.format(goname)
 
         field_name = xenlight_golang_fmt_name('{}_union'.format(keyname))
         s += 'x.{} = {}\n'.format(field_name, goname)
@@ -432,7 +432,7 @@ def xenlight_golang_array_from_C(ty = None):
         s += 'x.{}[i] = {}(v)\n'.format(goname, gotypename)
     else:
         s += 'if err := x.{}[i].fromC(&v); err != nil {{\n'.format(goname)
-        s += 'return err }\n'
+        s += 'return fmt.Errorf("converting field {}: %v", err) }}\n'.format(goname)
 
     s += '}\n}\n'
 
@@ -513,7 +513,7 @@ def xenlight_golang_convert_to_C(ty = None, outer_name = None,
     if not is_castable:
         s += 'if err := {}.{}.toC(&{}.{}); err != nil {{\n'.format(govarname,goname,
                                                                    cvarname,cname)
-        s += 'return err\n}\n'
+        s += 'return fmt.Errorf("converting field {}: %v", err) \n}}\n'.format(goname)
 
     elif gotypename == 'string':
         # Use the cgo helper for converting C strings.
@@ -615,7 +615,7 @@ def xenlight_golang_array_to_C(ty = None):
                                                                          golenvar,golenvar)
     s += 'for i,v := range x.{} {{\n'.format(goname)
     s += 'if err := v.toC(&c{}[i]); err != nil {{\n'.format(goname)
-    s += 'return err\n'
+    s += 'return fmt.Errorf("converting field {}: %v", err) \n'.format(goname)
     s += '}\n}\n}\n'
 
     return s
diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
index 889807d928..078c37f1c8 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -92,13 +92,13 @@ func (x *VgaInterfaceInfo) toC(xc *C.libxl_vga_interface_info) (err error) {
 
 func (x *VncInfo) fromC(xc *C.libxl_vnc_info) error {
 	if err := x.Enable.fromC(&xc.enable); err != nil {
-		return err
+		return fmt.Errorf("converting field Enable: %v", err)
 	}
 	x.Listen = C.GoString(xc.listen)
 	x.Passwd = C.GoString(xc.passwd)
 	x.Display = int(xc.display)
 	if err := x.Findunused.fromC(&xc.findunused); err != nil {
-		return err
+		return fmt.Errorf("converting field Findunused: %v", err)
 	}
 
 	return nil
@@ -112,7 +112,7 @@ func (x *VncInfo) toC(xc *C.libxl_vnc_info) (err error) {
 	}()
 
 	if err := x.Enable.toC(&xc.enable); err != nil {
-		return err
+		return fmt.Errorf("converting field Enable: %v", err)
 	}
 	if x.Listen != "" {
 		xc.listen = C.CString(x.Listen)
@@ -122,7 +122,7 @@ func (x *VncInfo) toC(xc *C.libxl_vnc_info) (err error) {
 	}
 	xc.display = C.int(x.Display)
 	if err := x.Findunused.toC(&xc.findunused); err != nil {
-		return err
+		return fmt.Errorf("converting field Findunused: %v", err)
 	}
 
 	return nil
@@ -130,23 +130,23 @@ func (x *VncInfo) toC(xc *C.libxl_vnc_info) (err error) {
 
 func (x *SpiceInfo) fromC(xc *C.libxl_spice_info) error {
 	if err := x.Enable.fromC(&xc.enable); err != nil {
-		return err
+		return fmt.Errorf("converting field Enable: %v", err)
 	}
 	x.Port = int(xc.port)
 	x.TlsPort = int(xc.tls_port)
 	x.Host = C.GoString(xc.host)
 	if err := x.DisableTicketing.fromC(&xc.disable_ticketing); err != nil {
-		return err
+		return fmt.Errorf("converting field DisableTicketing: %v", err)
 	}
 	x.Passwd = C.GoString(xc.passwd)
 	if err := x.AgentMouse.fromC(&xc.agent_mouse); err != nil {
-		return err
+		return fmt.Errorf("converting field AgentMouse: %v", err)
 	}
 	if err := x.Vdagent.fromC(&xc.vdagent); err != nil {
-		return err
+		return fmt.Errorf("converting field Vdagent: %v", err)
 	}
 	if err := x.ClipboardSharing.fromC(&xc.clipboard_sharing); err != nil {
-		return err
+		return fmt.Errorf("converting field ClipboardSharing: %v", err)
 	}
 	x.Usbredirection = int(xc.usbredirection)
 	x.ImageCompression = C.GoString(xc.image_compression)
@@ -163,7 +163,7 @@ func (x *SpiceInfo) toC(xc *C.libxl_spice_info) (err error) {
 	}()
 
 	if err := x.Enable.toC(&xc.enable); err != nil {
-		return err
+		return fmt.Errorf("converting field Enable: %v", err)
 	}
 	xc.port = C.int(x.Port)
 	xc.tls_port = C.int(x.TlsPort)
@@ -171,19 +171,19 @@ func (x *SpiceInfo) toC(xc *C.libxl_spice_info) (err error) {
 		xc.host = C.CString(x.Host)
 	}
 	if err := x.DisableTicketing.toC(&xc.disable_ticketing); err != nil {
-		return err
+		return fmt.Errorf("converting field DisableTicketing: %v", err)
 	}
 	if x.Passwd != "" {
 		xc.passwd = C.CString(x.Passwd)
 	}
 	if err := x.AgentMouse.toC(&xc.agent_mouse); err != nil {
-		return err
+		return fmt.Errorf("converting field AgentMouse: %v", err)
 	}
 	if err := x.Vdagent.toC(&xc.vdagent); err != nil {
-		return err
+		return fmt.Errorf("converting field Vdagent: %v", err)
 	}
 	if err := x.ClipboardSharing.toC(&xc.clipboard_sharing); err != nil {
-		return err
+		return fmt.Errorf("converting field ClipboardSharing: %v", err)
 	}
 	xc.usbredirection = C.int(x.Usbredirection)
 	if x.ImageCompression != "" {
@@ -198,10 +198,10 @@ func (x *SpiceInfo) toC(xc *C.libxl_spice_info) (err error) {
 
 func (x *SdlInfo) fromC(xc *C.libxl_sdl_info) error {
 	if err := x.Enable.fromC(&xc.enable); err != nil {
-		return err
+		return fmt.Errorf("converting field Enable: %v", err)
 	}
 	if err := x.Opengl.fromC(&xc.opengl); err != nil {
-		return err
+		return fmt.Errorf("converting field Opengl: %v", err)
 	}
 	x.Display = C.GoString(xc.display)
 	x.Xauthority = C.GoString(xc.xauthority)
@@ -217,10 +217,10 @@ func (x *SdlInfo) toC(xc *C.libxl_sdl_info) (err error) {
 	}()
 
 	if err := x.Enable.toC(&xc.enable); err != nil {
-		return err
+		return fmt.Errorf("converting field Enable: %v", err)
 	}
 	if err := x.Opengl.toC(&xc.opengl); err != nil {
-		return err
+		return fmt.Errorf("converting field Opengl: %v", err)
 	}
 	if x.Display != "" {
 		xc.display = C.CString(x.Display)
@@ -234,7 +234,7 @@ func (x *SdlInfo) toC(xc *C.libxl_sdl_info) (err error) {
 
 func (x *Dominfo) fromC(xc *C.libxl_dominfo) error {
 	if err := x.Uuid.fromC(&xc.uuid); err != nil {
-		return err
+		return fmt.Errorf("converting field Uuid: %v", err)
 	}
 	x.Domid = Domid(xc.domid)
 	x.Ssidref = uint32(xc.ssidref)
@@ -268,7 +268,7 @@ func (x *Dominfo) toC(xc *C.libxl_dominfo) (err error) {
 	}()
 
 	if err := x.Uuid.toC(&xc.uuid); err != nil {
-		return err
+		return fmt.Errorf("converting field Uuid: %v", err)
 	}
 	xc.domid = C.libxl_domid(x.Domid)
 	xc.ssidref = C.uint32_t(x.Ssidref)
@@ -302,7 +302,7 @@ func (x *Cpupoolinfo) fromC(xc *C.libxl_cpupoolinfo) error {
 	x.Sched = Scheduler(xc.sched)
 	x.NDom = uint32(xc.n_dom)
 	if err := x.Cpumap.fromC(&xc.cpumap); err != nil {
-		return err
+		return fmt.Errorf("converting field Cpumap: %v", err)
 	}
 
 	return nil
@@ -322,7 +322,7 @@ func (x *Cpupoolinfo) toC(xc *C.libxl_cpupoolinfo) (err error) {
 	xc.sched = C.libxl_scheduler(x.Sched)
 	xc.n_dom = C.uint32_t(x.NDom)
 	if err := x.Cpumap.toC(&xc.cpumap); err != nil {
-		return err
+		return fmt.Errorf("converting field Cpumap: %v", err)
 	}
 
 	return nil
@@ -342,7 +342,7 @@ func (x *Channelinfo) fromC(xc *C.libxl_channelinfo) error {
 	case ChannelConnectionPty:
 		var connectionPty ChannelinfoConnectionUnionPty
 		if err := connectionPty.fromC(xc); err != nil {
-			return err
+			return fmt.Errorf("converting field connectionPty: %v", err)
 		}
 		x.ConnectionUnion = connectionPty
 	default:
@@ -403,7 +403,7 @@ func (x *Channelinfo) toC(xc *C.libxl_channelinfo) (err error) {
 
 func (x *Vminfo) fromC(xc *C.libxl_vminfo) error {
 	if err := x.Uuid.fromC(&xc.uuid); err != nil {
-		return err
+		return fmt.Errorf("converting field Uuid: %v", err)
 	}
 	x.Domid = Domid(xc.domid)
 
@@ -418,7 +418,7 @@ func (x *Vminfo) toC(xc *C.libxl_vminfo) (err error) {
 	}()
 
 	if err := x.Uuid.toC(&xc.uuid); err != nil {
-		return err
+		return fmt.Errorf("converting field Uuid: %v", err)
 	}
 	xc.domid = C.libxl_domid(x.Domid)
 
@@ -488,30 +488,30 @@ func (x *VersionInfo) toC(xc *C.libxl_version_info) (err error) {
 func (x *DomainCreateInfo) fromC(xc *C.libxl_domain_create_info) error {
 	x.Type = DomainType(xc._type)
 	if err := x.Hap.fromC(&xc.hap); err != nil {
-		return err
+		return fmt.Errorf("converting field Hap: %v", err)
 	}
 	if err := x.Oos.fromC(&xc.oos); err != nil {
-		return err
+		return fmt.Errorf("converting field Oos: %v", err)
 	}
 	x.Ssidref = uint32(xc.ssidref)
 	x.SsidLabel = C.GoString(xc.ssid_label)
 	x.Name = C.GoString(xc.name)
 	if err := x.Uuid.fromC(&xc.uuid); err != nil {
-		return err
+		return fmt.Errorf("converting field Uuid: %v", err)
 	}
 	if err := x.Xsdata.fromC(&xc.xsdata); err != nil {
-		return err
+		return fmt.Errorf("converting field Xsdata: %v", err)
 	}
 	if err := x.Platformdata.fromC(&xc.platformdata); err != nil {
-		return err
+		return fmt.Errorf("converting field Platformdata: %v", err)
 	}
 	x.Poolid = uint32(xc.poolid)
 	x.PoolName = C.GoString(xc.pool_name)
 	if err := x.RunHotplugScripts.fromC(&xc.run_hotplug_scripts); err != nil {
-		return err
+		return fmt.Errorf("converting field RunHotplugScripts: %v", err)
 	}
 	if err := x.DriverDomain.fromC(&xc.driver_domain); err != nil {
-		return err
+		return fmt.Errorf("converting field DriverDomain: %v", err)
 	}
 	x.Passthrough = Passthrough(xc.passthrough)
 
@@ -527,10 +527,10 @@ func (x *DomainCreateInfo) toC(xc *C.libxl_domain_create_info) (err error) {
 
 	xc._type = C.libxl_domain_type(x.Type)
 	if err := x.Hap.toC(&xc.hap); err != nil {
-		return err
+		return fmt.Errorf("converting field Hap: %v", err)
 	}
 	if err := x.Oos.toC(&xc.oos); err != nil {
-		return err
+		return fmt.Errorf("converting field Oos: %v", err)
 	}
 	xc.ssidref = C.uint32_t(x.Ssidref)
 	if x.SsidLabel != "" {
@@ -540,23 +540,23 @@ func (x *DomainCreateInfo) toC(xc *C.libxl_domain_create_info) (err error) {
 		xc.name = C.CString(x.Name)
 	}
 	if err := x.Uuid.toC(&xc.uuid); err != nil {
-		return err
+		return fmt.Errorf("converting field Uuid: %v", err)
 	}
 	if err := x.Xsdata.toC(&xc.xsdata); err != nil {
-		return err
+		return fmt.Errorf("converting field Xsdata: %v", err)
 	}
 	if err := x.Platformdata.toC(&xc.platformdata); err != nil {
-		return err
+		return fmt.Errorf("converting field Platformdata: %v", err)
 	}
 	xc.poolid = C.uint32_t(x.Poolid)
 	if x.PoolName != "" {
 		xc.pool_name = C.CString(x.PoolName)
 	}
 	if err := x.RunHotplugScripts.toC(&xc.run_hotplug_scripts); err != nil {
-		return err
+		return fmt.Errorf("converting field RunHotplugScripts: %v", err)
 	}
 	if err := x.DriverDomain.toC(&xc.driver_domain); err != nil {
-		return err
+		return fmt.Errorf("converting field DriverDomain: %v", err)
 	}
 	xc.passthrough = C.libxl_passthrough(x.Passthrough)
 
@@ -568,7 +568,7 @@ func (x *DomainRestoreParams) fromC(xc *C.libxl_domain_restore_params) error {
 	x.StreamVersion = uint32(xc.stream_version)
 	x.ColoProxyScript = C.GoString(xc.colo_proxy_script)
 	if err := x.UserspaceColoProxy.fromC(&xc.userspace_colo_proxy); err != nil {
-		return err
+		return fmt.Errorf("converting field UserspaceColoProxy: %v", err)
 	}
 
 	return nil
@@ -587,7 +587,7 @@ func (x *DomainRestoreParams) toC(xc *C.libxl_domain_restore_params) (err error)
 		xc.colo_proxy_script = C.CString(x.ColoProxyScript)
 	}
 	if err := x.UserspaceColoProxy.toC(&xc.userspace_colo_proxy); err != nil {
-		return err
+		return fmt.Errorf("converting field UserspaceColoProxy: %v", err)
 	}
 
 	return nil
@@ -629,7 +629,7 @@ func (x *VcpuSchedParams) fromC(xc *C.libxl_vcpu_sched_params) error {
 		x.Vcpus = make([]SchedParams, numVcpus)
 		for i, v := range cVcpus {
 			if err := x.Vcpus[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Vcpus: %v", err)
 			}
 		}
 	}
@@ -651,7 +651,7 @@ func (x *VcpuSchedParams) toC(xc *C.libxl_vcpu_sched_params) (err error) {
 		cVcpus := (*[1 << 28]C.libxl_sched_params)(unsafe.Pointer(xc.vcpus))[:numVcpus:numVcpus]
 		for i, v := range x.Vcpus {
 			if err := v.toC(&cVcpus[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Vcpus: %v", err)
 			}
 		}
 	}
@@ -703,7 +703,7 @@ func (x *VnodeInfo) fromC(xc *C.libxl_vnode_info) error {
 	}
 	x.Pnode = uint32(xc.pnode)
 	if err := x.Vcpus.fromC(&xc.vcpus); err != nil {
-		return err
+		return fmt.Errorf("converting field Vcpus: %v", err)
 	}
 
 	return nil
@@ -727,7 +727,7 @@ func (x *VnodeInfo) toC(xc *C.libxl_vnode_info) (err error) {
 	}
 	xc.pnode = C.uint32_t(x.Pnode)
 	if err := x.Vcpus.toC(&xc.vcpus); err != nil {
-		return err
+		return fmt.Errorf("converting field Vcpus: %v", err)
 	}
 
 	return nil
@@ -756,13 +756,13 @@ func (x *RdmReserve) toC(xc *C.libxl_rdm_reserve) (err error) {
 func (x *DomainBuildInfo) fromC(xc *C.libxl_domain_build_info) error {
 	x.MaxVcpus = int(xc.max_vcpus)
 	if err := x.AvailVcpus.fromC(&xc.avail_vcpus); err != nil {
-		return err
+		return fmt.Errorf("converting field AvailVcpus: %v", err)
 	}
 	if err := x.Cpumap.fromC(&xc.cpumap); err != nil {
-		return err
+		return fmt.Errorf("converting field Cpumap: %v", err)
 	}
 	if err := x.Nodemap.fromC(&xc.nodemap); err != nil {
-		return err
+		return fmt.Errorf("converting field Nodemap: %v", err)
 	}
 	x.VcpuHardAffinity = nil
 	if numVcpuHardAffinity := int(xc.num_vcpu_hard_affinity); numVcpuHardAffinity > 0 {
@@ -770,7 +770,7 @@ func (x *DomainBuildInfo) fromC(xc *C.libxl_domain_build_info) error {
 		x.VcpuHardAffinity = make([]Bitmap, numVcpuHardAffinity)
 		for i, v := range cVcpuHardAffinity {
 			if err := x.VcpuHardAffinity[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field VcpuHardAffinity: %v", err)
 			}
 		}
 	}
@@ -780,12 +780,12 @@ func (x *DomainBuildInfo) fromC(xc *C.libxl_domain_build_info) error {
 		x.VcpuSoftAffinity = make([]Bitmap, numVcpuSoftAffinity)
 		for i, v := range cVcpuSoftAffinity {
 			if err := x.VcpuSoftAffinity[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field VcpuSoftAffinity: %v", err)
 			}
 		}
 	}
 	if err := x.NumaPlacement.fromC(&xc.numa_placement); err != nil {
-		return err
+		return fmt.Errorf("converting field NumaPlacement: %v", err)
 	}
 	x.TscMode = TscMode(xc.tsc_mode)
 	x.MaxMemkb = uint64(xc.max_memkb)
@@ -797,13 +797,13 @@ func (x *DomainBuildInfo) fromC(xc *C.libxl_domain_build_info) error {
 	x.ExecSsidref = uint32(xc.exec_ssidref)
 	x.ExecSsidLabel = C.GoString(xc.exec_ssid_label)
 	if err := x.Localtime.fromC(&xc.localtime); err != nil {
-		return err
+		return fmt.Errorf("converting field Localtime: %v", err)
 	}
 	if err := x.DisableMigrate.fromC(&xc.disable_migrate); err != nil {
-		return err
+		return fmt.Errorf("converting field DisableMigrate: %v", err)
 	}
 	if err := x.Cpuid.fromC(&xc.cpuid); err != nil {
-		return err
+		return fmt.Errorf("converting field Cpuid: %v", err)
 	}
 	x.BlkdevStart = C.GoString(xc.blkdev_start)
 	x.VnumaNodes = nil
@@ -812,7 +812,7 @@ func (x *DomainBuildInfo) fromC(xc *C.libxl_domain_build_info) error {
 		x.VnumaNodes = make([]VnodeInfo, numVnumaNodes)
 		for i, v := range cVnumaNodes {
 			if err := x.VnumaNodes[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field VnumaNodes: %v", err)
 			}
 		}
 	}
@@ -820,23 +820,23 @@ func (x *DomainBuildInfo) fromC(xc *C.libxl_domain_build_info) error {
 	x.MaxMaptrackFrames = uint32(xc.max_maptrack_frames)
 	x.DeviceModelVersion = DeviceModelVersion(xc.device_model_version)
 	if err := x.DeviceModelStubdomain.fromC(&xc.device_model_stubdomain); err != nil {
-		return err
+		return fmt.Errorf("converting field DeviceModelStubdomain: %v", err)
 	}
 	x.DeviceModel = C.GoString(xc.device_model)
 	x.DeviceModelSsidref = uint32(xc.device_model_ssidref)
 	x.DeviceModelSsidLabel = C.GoString(xc.device_model_ssid_label)
 	x.DeviceModelUser = C.GoString(xc.device_model_user)
 	if err := x.Extra.fromC(&xc.extra); err != nil {
-		return err
+		return fmt.Errorf("converting field Extra: %v", err)
 	}
 	if err := x.ExtraPv.fromC(&xc.extra_pv); err != nil {
-		return err
+		return fmt.Errorf("converting field ExtraPv: %v", err)
 	}
 	if err := x.ExtraHvm.fromC(&xc.extra_hvm); err != nil {
-		return err
+		return fmt.Errorf("converting field ExtraHvm: %v", err)
 	}
 	if err := x.SchedParams.fromC(&xc.sched_params); err != nil {
-		return err
+		return fmt.Errorf("converting field SchedParams: %v", err)
 	}
 	x.Ioports = nil
 	if numIoports := int(xc.num_ioports); numIoports > 0 {
@@ -844,7 +844,7 @@ func (x *DomainBuildInfo) fromC(xc *C.libxl_domain_build_info) error {
 		x.Ioports = make([]IoportRange, numIoports)
 		for i, v := range cIoports {
 			if err := x.Ioports[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Ioports: %v", err)
 			}
 		}
 	}
@@ -862,12 +862,12 @@ func (x *DomainBuildInfo) fromC(xc *C.libxl_domain_build_info) error {
 		x.Iomem = make([]IomemRange, numIomem)
 		for i, v := range cIomem {
 			if err := x.Iomem[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Iomem: %v", err)
 			}
 		}
 	}
 	if err := x.ClaimMode.fromC(&xc.claim_mode); err != nil {
-		return err
+		return fmt.Errorf("converting field ClaimMode: %v", err)
 	}
 	x.EventChannels = uint32(xc.event_channels)
 	x.Kernel = C.GoString(xc.kernel)
@@ -875,21 +875,21 @@ func (x *DomainBuildInfo) fromC(xc *C.libxl_domain_build_info) error {
 	x.Ramdisk = C.GoString(xc.ramdisk)
 	x.DeviceTree = C.GoString(xc.device_tree)
 	if err := x.Acpi.fromC(&xc.acpi); err != nil {
-		return err
+		return fmt.Errorf("converting field Acpi: %v", err)
 	}
 	x.Bootloader = C.GoString(xc.bootloader)
 	if err := x.BootloaderArgs.fromC(&xc.bootloader_args); err != nil {
-		return err
+		return fmt.Errorf("converting field BootloaderArgs: %v", err)
 	}
 	x.TimerMode = TimerMode(xc.timer_mode)
 	if err := x.NestedHvm.fromC(&xc.nested_hvm); err != nil {
-		return err
+		return fmt.Errorf("converting field NestedHvm: %v", err)
 	}
 	if err := x.Apic.fromC(&xc.apic); err != nil {
-		return err
+		return fmt.Errorf("converting field Apic: %v", err)
 	}
 	if err := x.DmRestrict.fromC(&xc.dm_restrict); err != nil {
-		return err
+		return fmt.Errorf("converting field DmRestrict: %v", err)
 	}
 	x.Tee = TeeType(xc.tee)
 	x.Type = DomainType(xc._type)
@@ -897,19 +897,19 @@ func (x *DomainBuildInfo) fromC(xc *C.libxl_domain_build_info) error {
 	case DomainTypeHvm:
 		var typeHvm DomainBuildInfoTypeUnionHvm
 		if err := typeHvm.fromC(xc); err != nil {
-			return err
+			return fmt.Errorf("converting field typeHvm: %v", err)
 		}
 		x.TypeUnion = typeHvm
 	case DomainTypePv:
 		var typePv DomainBuildInfoTypeUnionPv
 		if err := typePv.fromC(xc); err != nil {
-			return err
+			return fmt.Errorf("converting field typePv: %v", err)
 		}
 		x.TypeUnion = typePv
 	case DomainTypePvh:
 		var typePvh DomainBuildInfoTypeUnionPvh
 		if err := typePvh.fromC(xc); err != nil {
-			return err
+			return fmt.Errorf("converting field typePvh: %v", err)
 		}
 		x.TypeUnion = typePvh
 	default:
@@ -931,100 +931,100 @@ func (x *DomainBuildInfoTypeUnionHvm) fromC(xc *C.libxl_domain_build_info) error
 	x.Firmware = C.GoString(tmp.firmware)
 	x.Bios = BiosType(tmp.bios)
 	if err := x.Pae.fromC(&tmp.pae); err != nil {
-		return err
+		return fmt.Errorf("converting field Pae: %v", err)
 	}
 	if err := x.Apic.fromC(&tmp.apic); err != nil {
-		return err
+		return fmt.Errorf("converting field Apic: %v", err)
 	}
 	if err := x.Acpi.fromC(&tmp.acpi); err != nil {
-		return err
+		return fmt.Errorf("converting field Acpi: %v", err)
 	}
 	if err := x.AcpiS3.fromC(&tmp.acpi_s3); err != nil {
-		return err
+		return fmt.Errorf("converting field AcpiS3: %v", err)
 	}
 	if err := x.AcpiS4.fromC(&tmp.acpi_s4); err != nil {
-		return err
+		return fmt.Errorf("converting field AcpiS4: %v", err)
 	}
 	if err := x.AcpiLaptopSlate.fromC(&tmp.acpi_laptop_slate); err != nil {
-		return err
+		return fmt.Errorf("converting field AcpiLaptopSlate: %v", err)
 	}
 	if err := x.Nx.fromC(&tmp.nx); err != nil {
-		return err
+		return fmt.Errorf("converting field Nx: %v", err)
 	}
 	if err := x.Viridian.fromC(&tmp.viridian); err != nil {
-		return err
+		return fmt.Errorf("converting field Viridian: %v", err)
 	}
 	if err := x.ViridianEnable.fromC(&tmp.viridian_enable); err != nil {
-		return err
+		return fmt.Errorf("converting field ViridianEnable: %v", err)
 	}
 	if err := x.ViridianDisable.fromC(&tmp.viridian_disable); err != nil {
-		return err
+		return fmt.Errorf("converting field ViridianDisable: %v", err)
 	}
 	x.Timeoffset = C.GoString(tmp.timeoffset)
 	if err := x.Hpet.fromC(&tmp.hpet); err != nil {
-		return err
+		return fmt.Errorf("converting field Hpet: %v", err)
 	}
 	if err := x.VptAlign.fromC(&tmp.vpt_align); err != nil {
-		return err
+		return fmt.Errorf("converting field VptAlign: %v", err)
 	}
 	x.MmioHoleMemkb = uint64(tmp.mmio_hole_memkb)
 	x.TimerMode = TimerMode(tmp.timer_mode)
 	if err := x.NestedHvm.fromC(&tmp.nested_hvm); err != nil {
-		return err
+		return fmt.Errorf("converting field NestedHvm: %v", err)
 	}
 	if err := x.Altp2M.fromC(&tmp.altp2m); err != nil {
-		return err
+		return fmt.Errorf("converting field Altp2M: %v", err)
 	}
 	x.SystemFirmware = C.GoString(tmp.system_firmware)
 	x.SmbiosFirmware = C.GoString(tmp.smbios_firmware)
 	x.AcpiFirmware = C.GoString(tmp.acpi_firmware)
 	x.Hdtype = Hdtype(tmp.hdtype)
 	if err := x.Nographic.fromC(&tmp.nographic); err != nil {
-		return err
+		return fmt.Errorf("converting field Nographic: %v", err)
 	}
 	if err := x.Vga.fromC(&tmp.vga); err != nil {
-		return err
+		return fmt.Errorf("converting field Vga: %v", err)
 	}
 	if err := x.Vnc.fromC(&tmp.vnc); err != nil {
-		return err
+		return fmt.Errorf("converting field Vnc: %v", err)
 	}
 	x.Keymap = C.GoString(tmp.keymap)
 	if err := x.Sdl.fromC(&tmp.sdl); err != nil {
-		return err
+		return fmt.Errorf("converting field Sdl: %v", err)
 	}
 	if err := x.Spice.fromC(&tmp.spice); err != nil {
-		return err
+		return fmt.Errorf("converting field Spice: %v", err)
 	}
 	if err := x.GfxPassthru.fromC(&tmp.gfx_passthru); err != nil {
-		return err
+		return fmt.Errorf("converting field GfxPassthru: %v", err)
 	}
 	x.GfxPassthruKind = GfxPassthruKind(tmp.gfx_passthru_kind)
 	x.Serial = C.GoString(tmp.serial)
 	x.Boot = C.GoString(tmp.boot)
 	if err := x.Usb.fromC(&tmp.usb); err != nil {
-		return err
+		return fmt.Errorf("converting field Usb: %v", err)
 	}
 	x.Usbversion = int(tmp.usbversion)
 	x.Usbdevice = C.GoString(tmp.usbdevice)
 	if err := x.VkbDevice.fromC(&tmp.vkb_device); err != nil {
-		return err
+		return fmt.Errorf("converting field VkbDevice: %v", err)
 	}
 	x.Soundhw = C.GoString(tmp.soundhw)
 	if err := x.XenPlatformPci.fromC(&tmp.xen_platform_pci); err != nil {
-		return err
+		return fmt.Errorf("converting field XenPlatformPci: %v", err)
 	}
 	if err := x.UsbdeviceList.fromC(&tmp.usbdevice_list); err != nil {
-		return err
+		return fmt.Errorf("converting field UsbdeviceList: %v", err)
 	}
 	x.VendorDevice = VendorDevice(tmp.vendor_device)
 	if err := x.MsVmGenid.fromC(&tmp.ms_vm_genid); err != nil {
-		return err
+		return fmt.Errorf("converting field MsVmGenid: %v", err)
 	}
 	if err := x.SerialList.fromC(&tmp.serial_list); err != nil {
-		return err
+		return fmt.Errorf("converting field SerialList: %v", err)
 	}
 	if err := x.Rdm.fromC(&tmp.rdm); err != nil {
-		return err
+		return fmt.Errorf("converting field Rdm: %v", err)
 	}
 	x.RdmMemBoundaryMemkb = uint64(tmp.rdm_mem_boundary_memkb)
 	x.McaCaps = uint64(tmp.mca_caps)
@@ -1041,13 +1041,13 @@ func (x *DomainBuildInfoTypeUnionPv) fromC(xc *C.libxl_domain_build_info) error
 	x.SlackMemkb = uint64(tmp.slack_memkb)
 	x.Bootloader = C.GoString(tmp.bootloader)
 	if err := x.BootloaderArgs.fromC(&tmp.bootloader_args); err != nil {
-		return err
+		return fmt.Errorf("converting field BootloaderArgs: %v", err)
 	}
 	x.Cmdline = C.GoString(tmp.cmdline)
 	x.Ramdisk = C.GoString(tmp.ramdisk)
 	x.Features = C.GoString(tmp.features)
 	if err := x.E820Host.fromC(&tmp.e820_host); err != nil {
-		return err
+		return fmt.Errorf("converting field E820Host: %v", err)
 	}
 	return nil
 }
@@ -1059,7 +1059,7 @@ func (x *DomainBuildInfoTypeUnionPvh) fromC(xc *C.libxl_domain_build_info) error
 
 	tmp := (*C.libxl_domain_build_info_type_union_pvh)(unsafe.Pointer(&xc.u[0]))
 	if err := x.Pvshim.fromC(&tmp.pvshim); err != nil {
-		return err
+		return fmt.Errorf("converting field Pvshim: %v", err)
 	}
 	x.PvshimPath = C.GoString(tmp.pvshim_path)
 	x.PvshimCmdline = C.GoString(tmp.pvshim_cmdline)
@@ -1076,13 +1076,13 @@ func (x *DomainBuildInfo) toC(xc *C.libxl_domain_build_info) (err error) {
 
 	xc.max_vcpus = C.int(x.MaxVcpus)
 	if err := x.AvailVcpus.toC(&xc.avail_vcpus); err != nil {
-		return err
+		return fmt.Errorf("converting field AvailVcpus: %v", err)
 	}
 	if err := x.Cpumap.toC(&xc.cpumap); err != nil {
-		return err
+		return fmt.Errorf("converting field Cpumap: %v", err)
 	}
 	if err := x.Nodemap.toC(&xc.nodemap); err != nil {
-		return err
+		return fmt.Errorf("converting field Nodemap: %v", err)
 	}
 	if numVcpuHardAffinity := len(x.VcpuHardAffinity); numVcpuHardAffinity > 0 {
 		xc.vcpu_hard_affinity = (*C.libxl_bitmap)(C.malloc(C.ulong(numVcpuHardAffinity) * C.sizeof_libxl_bitmap))
@@ -1090,7 +1090,7 @@ func (x *DomainBuildInfo) toC(xc *C.libxl_domain_build_info) (err error) {
 		cVcpuHardAffinity := (*[1 << 28]C.libxl_bitmap)(unsafe.Pointer(xc.vcpu_hard_affinity))[:numVcpuHardAffinity:numVcpuHardAffinity]
 		for i, v := range x.VcpuHardAffinity {
 			if err := v.toC(&cVcpuHardAffinity[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field VcpuHardAffinity: %v", err)
 			}
 		}
 	}
@@ -1100,12 +1100,12 @@ func (x *DomainBuildInfo) toC(xc *C.libxl_domain_build_info) (err error) {
 		cVcpuSoftAffinity := (*[1 << 28]C.libxl_bitmap)(unsafe.Pointer(xc.vcpu_soft_affinity))[:numVcpuSoftAffinity:numVcpuSoftAffinity]
 		for i, v := range x.VcpuSoftAffinity {
 			if err := v.toC(&cVcpuSoftAffinity[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field VcpuSoftAffinity: %v", err)
 			}
 		}
 	}
 	if err := x.NumaPlacement.toC(&xc.numa_placement); err != nil {
-		return err
+		return fmt.Errorf("converting field NumaPlacement: %v", err)
 	}
 	xc.tsc_mode = C.libxl_tsc_mode(x.TscMode)
 	xc.max_memkb = C.uint64_t(x.MaxMemkb)
@@ -1119,13 +1119,13 @@ func (x *DomainBuildInfo) toC(xc *C.libxl_domain_build_info) (err error) {
 		xc.exec_ssid_label = C.CString(x.ExecSsidLabel)
 	}
 	if err := x.Localtime.toC(&xc.localtime); err != nil {
-		return err
+		return fmt.Errorf("converting field Localtime: %v", err)
 	}
 	if err := x.DisableMigrate.toC(&xc.disable_migrate); err != nil {
-		return err
+		return fmt.Errorf("converting field DisableMigrate: %v", err)
 	}
 	if err := x.Cpuid.toC(&xc.cpuid); err != nil {
-		return err
+		return fmt.Errorf("converting field Cpuid: %v", err)
 	}
 	if x.BlkdevStart != "" {
 		xc.blkdev_start = C.CString(x.BlkdevStart)
@@ -1136,7 +1136,7 @@ func (x *DomainBuildInfo) toC(xc *C.libxl_domain_build_info) (err error) {
 		cVnumaNodes := (*[1 << 28]C.libxl_vnode_info)(unsafe.Pointer(xc.vnuma_nodes))[:numVnumaNodes:numVnumaNodes]
 		for i, v := range x.VnumaNodes {
 			if err := v.toC(&cVnumaNodes[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field VnumaNodes: %v", err)
 			}
 		}
 	}
@@ -1144,7 +1144,7 @@ func (x *DomainBuildInfo) toC(xc *C.libxl_domain_build_info) (err error) {
 	xc.max_maptrack_frames = C.uint32_t(x.MaxMaptrackFrames)
 	xc.device_model_version = C.libxl_device_model_version(x.DeviceModelVersion)
 	if err := x.DeviceModelStubdomain.toC(&xc.device_model_stubdomain); err != nil {
-		return err
+		return fmt.Errorf("converting field DeviceModelStubdomain: %v", err)
 	}
 	if x.DeviceModel != "" {
 		xc.device_model = C.CString(x.DeviceModel)
@@ -1157,16 +1157,16 @@ func (x *DomainBuildInfo) toC(xc *C.libxl_domain_build_info) (err error) {
 		xc.device_model_user = C.CString(x.DeviceModelUser)
 	}
 	if err := x.Extra.toC(&xc.extra); err != nil {
-		return err
+		return fmt.Errorf("converting field Extra: %v", err)
 	}
 	if err := x.ExtraPv.toC(&xc.extra_pv); err != nil {
-		return err
+		return fmt.Errorf("converting field ExtraPv: %v", err)
 	}
 	if err := x.ExtraHvm.toC(&xc.extra_hvm); err != nil {
-		return err
+		return fmt.Errorf("converting field ExtraHvm: %v", err)
 	}
 	if err := x.SchedParams.toC(&xc.sched_params); err != nil {
-		return err
+		return fmt.Errorf("converting field SchedParams: %v", err)
 	}
 	if numIoports := len(x.Ioports); numIoports > 0 {
 		xc.ioports = (*C.libxl_ioport_range)(C.malloc(C.ulong(numIoports) * C.sizeof_libxl_ioport_range))
@@ -1174,7 +1174,7 @@ func (x *DomainBuildInfo) toC(xc *C.libxl_domain_build_info) (err error) {
 		cIoports := (*[1 << 28]C.libxl_ioport_range)(unsafe.Pointer(xc.ioports))[:numIoports:numIoports]
 		for i, v := range x.Ioports {
 			if err := v.toC(&cIoports[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Ioports: %v", err)
 			}
 		}
 	}
@@ -1192,12 +1192,12 @@ func (x *DomainBuildInfo) toC(xc *C.libxl_domain_build_info) (err error) {
 		cIomem := (*[1 << 28]C.libxl_iomem_range)(unsafe.Pointer(xc.iomem))[:numIomem:numIomem]
 		for i, v := range x.Iomem {
 			if err := v.toC(&cIomem[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Iomem: %v", err)
 			}
 		}
 	}
 	if err := x.ClaimMode.toC(&xc.claim_mode); err != nil {
-		return err
+		return fmt.Errorf("converting field ClaimMode: %v", err)
 	}
 	xc.event_channels = C.uint32_t(x.EventChannels)
 	if x.Kernel != "" {
@@ -1213,23 +1213,23 @@ func (x *DomainBuildInfo) toC(xc *C.libxl_domain_build_info) (err error) {
 		xc.device_tree = C.CString(x.DeviceTree)
 	}
 	if err := x.Acpi.toC(&xc.acpi); err != nil {
-		return err
+		return fmt.Errorf("converting field Acpi: %v", err)
 	}
 	if x.Bootloader != "" {
 		xc.bootloader = C.CString(x.Bootloader)
 	}
 	if err := x.BootloaderArgs.toC(&xc.bootloader_args); err != nil {
-		return err
+		return fmt.Errorf("converting field BootloaderArgs: %v", err)
 	}
 	xc.timer_mode = C.libxl_timer_mode(x.TimerMode)
 	if err := x.NestedHvm.toC(&xc.nested_hvm); err != nil {
-		return err
+		return fmt.Errorf("converting field NestedHvm: %v", err)
 	}
 	if err := x.Apic.toC(&xc.apic); err != nil {
-		return err
+		return fmt.Errorf("converting field Apic: %v", err)
 	}
 	if err := x.DmRestrict.toC(&xc.dm_restrict); err != nil {
-		return err
+		return fmt.Errorf("converting field DmRestrict: %v", err)
 	}
 	xc.tee = C.libxl_tee_type(x.Tee)
 	xc._type = C.libxl_domain_type(x.Type)
@@ -1245,51 +1245,51 @@ func (x *DomainBuildInfo) toC(xc *C.libxl_domain_build_info) (err error) {
 		}
 		hvm.bios = C.libxl_bios_type(tmp.Bios)
 		if err := tmp.Pae.toC(&hvm.pae); err != nil {
-			return err
+			return fmt.Errorf("converting field Pae: %v", err)
 		}
 		if err := tmp.Apic.toC(&hvm.apic); err != nil {
-			return err
+			return fmt.Errorf("converting field Apic: %v", err)
 		}
 		if err := tmp.Acpi.toC(&hvm.acpi); err != nil {
-			return err
+			return fmt.Errorf("converting field Acpi: %v", err)
 		}
 		if err := tmp.AcpiS3.toC(&hvm.acpi_s3); err != nil {
-			return err
+			return fmt.Errorf("converting field AcpiS3: %v", err)
 		}
 		if err := tmp.AcpiS4.toC(&hvm.acpi_s4); err != nil {
-			return err
+			return fmt.Errorf("converting field AcpiS4: %v", err)
 		}
 		if err := tmp.AcpiLaptopSlate.toC(&hvm.acpi_laptop_slate); err != nil {
-			return err
+			return fmt.Errorf("converting field AcpiLaptopSlate: %v", err)
 		}
 		if err := tmp.Nx.toC(&hvm.nx); err != nil {
-			return err
+			return fmt.Errorf("converting field Nx: %v", err)
 		}
 		if err := tmp.Viridian.toC(&hvm.viridian); err != nil {
-			return err
+			return fmt.Errorf("converting field Viridian: %v", err)
 		}
 		if err := tmp.ViridianEnable.toC(&hvm.viridian_enable); err != nil {
-			return err
+			return fmt.Errorf("converting field ViridianEnable: %v", err)
 		}
 		if err := tmp.ViridianDisable.toC(&hvm.viridian_disable); err != nil {
-			return err
+			return fmt.Errorf("converting field ViridianDisable: %v", err)
 		}
 		if tmp.Timeoffset != "" {
 			hvm.timeoffset = C.CString(tmp.Timeoffset)
 		}
 		if err := tmp.Hpet.toC(&hvm.hpet); err != nil {
-			return err
+			return fmt.Errorf("converting field Hpet: %v", err)
 		}
 		if err := tmp.VptAlign.toC(&hvm.vpt_align); err != nil {
-			return err
+			return fmt.Errorf("converting field VptAlign: %v", err)
 		}
 		hvm.mmio_hole_memkb = C.uint64_t(tmp.MmioHoleMemkb)
 		hvm.timer_mode = C.libxl_timer_mode(tmp.TimerMode)
 		if err := tmp.NestedHvm.toC(&hvm.nested_hvm); err != nil {
-			return err
+			return fmt.Errorf("converting field NestedHvm: %v", err)
 		}
 		if err := tmp.Altp2M.toC(&hvm.altp2m); err != nil {
-			return err
+			return fmt.Errorf("converting field Altp2M: %v", err)
 		}
 		if tmp.SystemFirmware != "" {
 			hvm.system_firmware = C.CString(tmp.SystemFirmware)
@@ -1302,25 +1302,25 @@ func (x *DomainBuildInfo) toC(xc *C.libxl_domain_build_info) (err error) {
 		}
 		hvm.hdtype = C.libxl_hdtype(tmp.Hdtype)
 		if err := tmp.Nographic.toC(&hvm.nographic); err != nil {
-			return err
+			return fmt.Errorf("converting field Nographic: %v", err)
 		}
 		if err := tmp.Vga.toC(&hvm.vga); err != nil {
-			return err
+			return fmt.Errorf("converting field Vga: %v", err)
 		}
 		if err := tmp.Vnc.toC(&hvm.vnc); err != nil {
-			return err
+			return fmt.Errorf("converting field Vnc: %v", err)
 		}
 		if tmp.Keymap != "" {
 			hvm.keymap = C.CString(tmp.Keymap)
 		}
 		if err := tmp.Sdl.toC(&hvm.sdl); err != nil {
-			return err
+			return fmt.Errorf("converting field Sdl: %v", err)
 		}
 		if err := tmp.Spice.toC(&hvm.spice); err != nil {
-			return err
+			return fmt.Errorf("converting field Spice: %v", err)
 		}
 		if err := tmp.GfxPassthru.toC(&hvm.gfx_passthru); err != nil {
-			return err
+			return fmt.Errorf("converting field GfxPassthru: %v", err)
 		}
 		hvm.gfx_passthru_kind = C.libxl_gfx_passthru_kind(tmp.GfxPassthruKind)
 		if tmp.Serial != "" {
@@ -1330,33 +1330,33 @@ func (x *DomainBuildInfo) toC(xc *C.libxl_domain_build_info) (err error) {
 			hvm.boot = C.CString(tmp.Boot)
 		}
 		if err := tmp.Usb.toC(&hvm.usb); err != nil {
-			return err
+			return fmt.Errorf("converting field Usb: %v", err)
 		}
 		hvm.usbversion = C.int(tmp.Usbversion)
 		if tmp.Usbdevice != "" {
 			hvm.usbdevice = C.CString(tmp.Usbdevice)
 		}
 		if err := tmp.VkbDevice.toC(&hvm.vkb_device); err != nil {
-			return err
+			return fmt.Errorf("converting field VkbDevice: %v", err)
 		}
 		if tmp.Soundhw != "" {
 			hvm.soundhw = C.CString(tmp.Soundhw)
 		}
 		if err := tmp.XenPlatformPci.toC(&hvm.xen_platform_pci); err != nil {
-			return err
+			return fmt.Errorf("converting field XenPlatformPci: %v", err)
 		}
 		if err := tmp.UsbdeviceList.toC(&hvm.usbdevice_list); err != nil {
-			return err
+			return fmt.Errorf("converting field UsbdeviceList: %v", err)
 		}
 		hvm.vendor_device = C.libxl_vendor_device(tmp.VendorDevice)
 		if err := tmp.MsVmGenid.toC(&hvm.ms_vm_genid); err != nil {
-			return err
+			return fmt.Errorf("converting field MsVmGenid: %v", err)
 		}
 		if err := tmp.SerialList.toC(&hvm.serial_list); err != nil {
-			return err
+			return fmt.Errorf("converting field SerialList: %v", err)
 		}
 		if err := tmp.Rdm.toC(&hvm.rdm); err != nil {
-			return err
+			return fmt.Errorf("converting field Rdm: %v", err)
 		}
 		hvm.rdm_mem_boundary_memkb = C.uint64_t(tmp.RdmMemBoundaryMemkb)
 		hvm.mca_caps = C.uint64_t(tmp.McaCaps)
@@ -1376,7 +1376,7 @@ func (x *DomainBuildInfo) toC(xc *C.libxl_domain_build_info) (err error) {
 			pv.bootloader = C.CString(tmp.Bootloader)
 		}
 		if err := tmp.BootloaderArgs.toC(&pv.bootloader_args); err != nil {
-			return err
+			return fmt.Errorf("converting field BootloaderArgs: %v", err)
 		}
 		if tmp.Cmdline != "" {
 			pv.cmdline = C.CString(tmp.Cmdline)
@@ -1388,7 +1388,7 @@ func (x *DomainBuildInfo) toC(xc *C.libxl_domain_build_info) (err error) {
 			pv.features = C.CString(tmp.Features)
 		}
 		if err := tmp.E820Host.toC(&pv.e820_host); err != nil {
-			return err
+			return fmt.Errorf("converting field E820Host: %v", err)
 		}
 		pvBytes := C.GoBytes(unsafe.Pointer(&pv), C.sizeof_libxl_domain_build_info_type_union_pv)
 		copy(xc.u[:], pvBytes)
@@ -1399,7 +1399,7 @@ func (x *DomainBuildInfo) toC(xc *C.libxl_domain_build_info) (err error) {
 		}
 		var pvh C.libxl_domain_build_info_type_union_pvh
 		if err := tmp.Pvshim.toC(&pvh.pvshim); err != nil {
-			return err
+			return fmt.Errorf("converting field Pvshim: %v", err)
 		}
 		if tmp.PvshimPath != "" {
 			pvh.pvshim_path = C.CString(tmp.PvshimPath)
@@ -1427,10 +1427,10 @@ func (x *DeviceVfb) fromC(xc *C.libxl_device_vfb) error {
 	x.BackendDomname = C.GoString(xc.backend_domname)
 	x.Devid = Devid(xc.devid)
 	if err := x.Vnc.fromC(&xc.vnc); err != nil {
-		return err
+		return fmt.Errorf("converting field Vnc: %v", err)
 	}
 	if err := x.Sdl.fromC(&xc.sdl); err != nil {
-		return err
+		return fmt.Errorf("converting field Sdl: %v", err)
 	}
 	x.Keymap = C.GoString(xc.keymap)
 
@@ -1450,10 +1450,10 @@ func (x *DeviceVfb) toC(xc *C.libxl_device_vfb) (err error) {
 	}
 	xc.devid = C.libxl_devid(x.Devid)
 	if err := x.Vnc.toC(&xc.vnc); err != nil {
-		return err
+		return fmt.Errorf("converting field Vnc: %v", err)
 	}
 	if err := x.Sdl.toC(&xc.sdl); err != nil {
-		return err
+		return fmt.Errorf("converting field Sdl: %v", err)
 	}
 	if x.Keymap != "" {
 		xc.keymap = C.CString(x.Keymap)
@@ -1525,13 +1525,13 @@ func (x *DeviceDisk) fromC(xc *C.libxl_device_disk) error {
 	x.IsCdrom = int(xc.is_cdrom)
 	x.DirectIoSafe = bool(xc.direct_io_safe)
 	if err := x.DiscardEnable.fromC(&xc.discard_enable); err != nil {
-		return err
+		return fmt.Errorf("converting field DiscardEnable: %v", err)
 	}
 	if err := x.ColoEnable.fromC(&xc.colo_enable); err != nil {
-		return err
+		return fmt.Errorf("converting field ColoEnable: %v", err)
 	}
 	if err := x.ColoRestoreEnable.fromC(&xc.colo_restore_enable); err != nil {
-		return err
+		return fmt.Errorf("converting field ColoRestoreEnable: %v", err)
 	}
 	x.ColoHost = C.GoString(xc.colo_host)
 	x.ColoPort = int(xc.colo_port)
@@ -1569,13 +1569,13 @@ func (x *DeviceDisk) toC(xc *C.libxl_device_disk) (err error) {
 	xc.is_cdrom = C.int(x.IsCdrom)
 	xc.direct_io_safe = C.bool(x.DirectIoSafe)
 	if err := x.DiscardEnable.toC(&xc.discard_enable); err != nil {
-		return err
+		return fmt.Errorf("converting field DiscardEnable: %v", err)
 	}
 	if err := x.ColoEnable.toC(&xc.colo_enable); err != nil {
-		return err
+		return fmt.Errorf("converting field ColoEnable: %v", err)
 	}
 	if err := x.ColoRestoreEnable.toC(&xc.colo_restore_enable); err != nil {
-		return err
+		return fmt.Errorf("converting field ColoRestoreEnable: %v", err)
 	}
 	if x.ColoHost != "" {
 		xc.colo_host = C.CString(x.ColoHost)
@@ -1601,7 +1601,7 @@ func (x *DeviceNic) fromC(xc *C.libxl_device_nic) error {
 	x.Mtu = int(xc.mtu)
 	x.Model = C.GoString(xc.model)
 	if err := x.Mac.fromC(&xc.mac); err != nil {
-		return err
+		return fmt.Errorf("converting field Mac: %v", err)
 	}
 	x.Ip = C.GoString(xc.ip)
 	x.Bridge = C.GoString(xc.bridge)
@@ -1681,7 +1681,7 @@ func (x *DeviceNic) toC(xc *C.libxl_device_nic) (err error) {
 		xc.model = C.CString(x.Model)
 	}
 	if err := x.Mac.toC(&xc.mac); err != nil {
-		return err
+		return fmt.Errorf("converting field Mac: %v", err)
 	}
 	if x.Ip != "" {
 		xc.ip = C.CString(x.Ip)
@@ -1950,7 +1950,7 @@ func (x *DeviceUsbdev) fromC(xc *C.libxl_device_usbdev) error {
 	case UsbdevTypeHostdev:
 		var typeHostdev DeviceUsbdevTypeUnionHostdev
 		if err := typeHostdev.fromC(xc); err != nil {
-			return err
+			return fmt.Errorf("converting field typeHostdev: %v", err)
 		}
 		x.TypeUnion = typeHostdev
 	default:
@@ -2024,7 +2024,7 @@ func (x *DeviceVtpm) fromC(xc *C.libxl_device_vtpm) error {
 	x.BackendDomname = C.GoString(xc.backend_domname)
 	x.Devid = Devid(xc.devid)
 	if err := x.Uuid.fromC(&xc.uuid); err != nil {
-		return err
+		return fmt.Errorf("converting field Uuid: %v", err)
 	}
 
 	return nil
@@ -2043,7 +2043,7 @@ func (x *DeviceVtpm) toC(xc *C.libxl_device_vtpm) (err error) {
 	}
 	xc.devid = C.libxl_devid(x.Devid)
 	if err := x.Uuid.toC(&xc.uuid); err != nil {
-		return err
+		return fmt.Errorf("converting field Uuid: %v", err)
 	}
 
 	return nil
@@ -2119,7 +2119,7 @@ func (x *DeviceChannel) fromC(xc *C.libxl_device_channel) error {
 	case ChannelConnectionSocket:
 		var connectionSocket DeviceChannelConnectionUnionSocket
 		if err := connectionSocket.fromC(xc); err != nil {
-			return err
+			return fmt.Errorf("converting field connectionSocket: %v", err)
 		}
 		x.ConnectionUnion = connectionSocket
 	default:
@@ -2209,7 +2209,7 @@ func (x *DeviceVdispl) fromC(xc *C.libxl_device_vdispl) error {
 		x.Connectors = make([]ConnectorParam, numConnectors)
 		for i, v := range cConnectors {
 			if err := x.Connectors[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Connectors: %v", err)
 			}
 		}
 	}
@@ -2236,7 +2236,7 @@ func (x *DeviceVdispl) toC(xc *C.libxl_device_vdispl) (err error) {
 		cConnectors := (*[1 << 28]C.libxl_connector_param)(unsafe.Pointer(xc.connectors))[:numConnectors:numConnectors]
 		for i, v := range x.Connectors {
 			if err := v.toC(&cConnectors[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Connectors: %v", err)
 			}
 		}
 	}
@@ -2302,7 +2302,7 @@ func (x *VsndStream) fromC(xc *C.libxl_vsnd_stream) error {
 	x.UniqueId = C.GoString(xc.unique_id)
 	x.Type = VsndStreamType(xc._type)
 	if err := x.Params.fromC(&xc.params); err != nil {
-		return err
+		return fmt.Errorf("converting field Params: %v", err)
 	}
 
 	return nil
@@ -2320,7 +2320,7 @@ func (x *VsndStream) toC(xc *C.libxl_vsnd_stream) (err error) {
 	}
 	xc._type = C.libxl_vsnd_stream_type(x.Type)
 	if err := x.Params.toC(&xc.params); err != nil {
-		return err
+		return fmt.Errorf("converting field Params: %v", err)
 	}
 
 	return nil
@@ -2329,7 +2329,7 @@ func (x *VsndStream) toC(xc *C.libxl_vsnd_stream) (err error) {
 func (x *VsndPcm) fromC(xc *C.libxl_vsnd_pcm) error {
 	x.Name = C.GoString(xc.name)
 	if err := x.Params.fromC(&xc.params); err != nil {
-		return err
+		return fmt.Errorf("converting field Params: %v", err)
 	}
 	x.Streams = nil
 	if numVsndStreams := int(xc.num_vsnd_streams); numVsndStreams > 0 {
@@ -2337,7 +2337,7 @@ func (x *VsndPcm) fromC(xc *C.libxl_vsnd_pcm) error {
 		x.Streams = make([]VsndStream, numVsndStreams)
 		for i, v := range cStreams {
 			if err := x.Streams[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Streams: %v", err)
 			}
 		}
 	}
@@ -2356,7 +2356,7 @@ func (x *VsndPcm) toC(xc *C.libxl_vsnd_pcm) (err error) {
 		xc.name = C.CString(x.Name)
 	}
 	if err := x.Params.toC(&xc.params); err != nil {
-		return err
+		return fmt.Errorf("converting field Params: %v", err)
 	}
 	if numVsndStreams := len(x.Streams); numVsndStreams > 0 {
 		xc.streams = (*C.libxl_vsnd_stream)(C.malloc(C.ulong(numVsndStreams) * C.sizeof_libxl_vsnd_stream))
@@ -2364,7 +2364,7 @@ func (x *VsndPcm) toC(xc *C.libxl_vsnd_pcm) (err error) {
 		cStreams := (*[1 << 28]C.libxl_vsnd_stream)(unsafe.Pointer(xc.streams))[:numVsndStreams:numVsndStreams]
 		for i, v := range x.Streams {
 			if err := v.toC(&cStreams[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Streams: %v", err)
 			}
 		}
 	}
@@ -2379,7 +2379,7 @@ func (x *DeviceVsnd) fromC(xc *C.libxl_device_vsnd) error {
 	x.ShortName = C.GoString(xc.short_name)
 	x.LongName = C.GoString(xc.long_name)
 	if err := x.Params.fromC(&xc.params); err != nil {
-		return err
+		return fmt.Errorf("converting field Params: %v", err)
 	}
 	x.Pcms = nil
 	if numVsndPcms := int(xc.num_vsnd_pcms); numVsndPcms > 0 {
@@ -2387,7 +2387,7 @@ func (x *DeviceVsnd) fromC(xc *C.libxl_device_vsnd) error {
 		x.Pcms = make([]VsndPcm, numVsndPcms)
 		for i, v := range cPcms {
 			if err := x.Pcms[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Pcms: %v", err)
 			}
 		}
 	}
@@ -2414,7 +2414,7 @@ func (x *DeviceVsnd) toC(xc *C.libxl_device_vsnd) (err error) {
 		xc.long_name = C.CString(x.LongName)
 	}
 	if err := x.Params.toC(&xc.params); err != nil {
-		return err
+		return fmt.Errorf("converting field Params: %v", err)
 	}
 	if numVsndPcms := len(x.Pcms); numVsndPcms > 0 {
 		xc.pcms = (*C.libxl_vsnd_pcm)(C.malloc(C.ulong(numVsndPcms) * C.sizeof_libxl_vsnd_pcm))
@@ -2422,7 +2422,7 @@ func (x *DeviceVsnd) toC(xc *C.libxl_device_vsnd) (err error) {
 		cPcms := (*[1 << 28]C.libxl_vsnd_pcm)(unsafe.Pointer(xc.pcms))[:numVsndPcms:numVsndPcms]
 		for i, v := range x.Pcms {
 			if err := v.toC(&cPcms[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Pcms: %v", err)
 			}
 		}
 	}
@@ -2432,10 +2432,10 @@ func (x *DeviceVsnd) toC(xc *C.libxl_device_vsnd) (err error) {
 
 func (x *DomainConfig) fromC(xc *C.libxl_domain_config) error {
 	if err := x.CInfo.fromC(&xc.c_info); err != nil {
-		return err
+		return fmt.Errorf("converting field CInfo: %v", err)
 	}
 	if err := x.BInfo.fromC(&xc.b_info); err != nil {
-		return err
+		return fmt.Errorf("converting field BInfo: %v", err)
 	}
 	x.Disks = nil
 	if numDisks := int(xc.num_disks); numDisks > 0 {
@@ -2443,7 +2443,7 @@ func (x *DomainConfig) fromC(xc *C.libxl_domain_config) error {
 		x.Disks = make([]DeviceDisk, numDisks)
 		for i, v := range cDisks {
 			if err := x.Disks[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Disks: %v", err)
 			}
 		}
 	}
@@ -2453,7 +2453,7 @@ func (x *DomainConfig) fromC(xc *C.libxl_domain_config) error {
 		x.Nics = make([]DeviceNic, numNics)
 		for i, v := range cNics {
 			if err := x.Nics[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Nics: %v", err)
 			}
 		}
 	}
@@ -2463,7 +2463,7 @@ func (x *DomainConfig) fromC(xc *C.libxl_domain_config) error {
 		x.Pcidevs = make([]DevicePci, numPcidevs)
 		for i, v := range cPcidevs {
 			if err := x.Pcidevs[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Pcidevs: %v", err)
 			}
 		}
 	}
@@ -2473,7 +2473,7 @@ func (x *DomainConfig) fromC(xc *C.libxl_domain_config) error {
 		x.Rdms = make([]DeviceRdm, numRdms)
 		for i, v := range cRdms {
 			if err := x.Rdms[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Rdms: %v", err)
 			}
 		}
 	}
@@ -2483,7 +2483,7 @@ func (x *DomainConfig) fromC(xc *C.libxl_domain_config) error {
 		x.Dtdevs = make([]DeviceDtdev, numDtdevs)
 		for i, v := range cDtdevs {
 			if err := x.Dtdevs[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Dtdevs: %v", err)
 			}
 		}
 	}
@@ -2493,7 +2493,7 @@ func (x *DomainConfig) fromC(xc *C.libxl_domain_config) error {
 		x.Vfbs = make([]DeviceVfb, numVfbs)
 		for i, v := range cVfbs {
 			if err := x.Vfbs[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Vfbs: %v", err)
 			}
 		}
 	}
@@ -2503,7 +2503,7 @@ func (x *DomainConfig) fromC(xc *C.libxl_domain_config) error {
 		x.Vkbs = make([]DeviceVkb, numVkbs)
 		for i, v := range cVkbs {
 			if err := x.Vkbs[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Vkbs: %v", err)
 			}
 		}
 	}
@@ -2513,7 +2513,7 @@ func (x *DomainConfig) fromC(xc *C.libxl_domain_config) error {
 		x.Vtpms = make([]DeviceVtpm, numVtpms)
 		for i, v := range cVtpms {
 			if err := x.Vtpms[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Vtpms: %v", err)
 			}
 		}
 	}
@@ -2523,7 +2523,7 @@ func (x *DomainConfig) fromC(xc *C.libxl_domain_config) error {
 		x.P9S = make([]DeviceP9, numP9S)
 		for i, v := range cP9S {
 			if err := x.P9S[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field P9S: %v", err)
 			}
 		}
 	}
@@ -2533,7 +2533,7 @@ func (x *DomainConfig) fromC(xc *C.libxl_domain_config) error {
 		x.Pvcallsifs = make([]DevicePvcallsif, numPvcallsifs)
 		for i, v := range cPvcallsifs {
 			if err := x.Pvcallsifs[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Pvcallsifs: %v", err)
 			}
 		}
 	}
@@ -2543,7 +2543,7 @@ func (x *DomainConfig) fromC(xc *C.libxl_domain_config) error {
 		x.Vdispls = make([]DeviceVdispl, numVdispls)
 		for i, v := range cVdispls {
 			if err := x.Vdispls[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Vdispls: %v", err)
 			}
 		}
 	}
@@ -2553,7 +2553,7 @@ func (x *DomainConfig) fromC(xc *C.libxl_domain_config) error {
 		x.Vsnds = make([]DeviceVsnd, numVsnds)
 		for i, v := range cVsnds {
 			if err := x.Vsnds[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Vsnds: %v", err)
 			}
 		}
 	}
@@ -2563,7 +2563,7 @@ func (x *DomainConfig) fromC(xc *C.libxl_domain_config) error {
 		x.Channels = make([]DeviceChannel, numChannels)
 		for i, v := range cChannels {
 			if err := x.Channels[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Channels: %v", err)
 			}
 		}
 	}
@@ -2573,7 +2573,7 @@ func (x *DomainConfig) fromC(xc *C.libxl_domain_config) error {
 		x.Usbctrls = make([]DeviceUsbctrl, numUsbctrls)
 		for i, v := range cUsbctrls {
 			if err := x.Usbctrls[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Usbctrls: %v", err)
 			}
 		}
 	}
@@ -2583,7 +2583,7 @@ func (x *DomainConfig) fromC(xc *C.libxl_domain_config) error {
 		x.Usbdevs = make([]DeviceUsbdev, numUsbdevs)
 		for i, v := range cUsbdevs {
 			if err := x.Usbdevs[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Usbdevs: %v", err)
 			}
 		}
 	}
@@ -2604,10 +2604,10 @@ func (x *DomainConfig) toC(xc *C.libxl_domain_config) (err error) {
 	}()
 
 	if err := x.CInfo.toC(&xc.c_info); err != nil {
-		return err
+		return fmt.Errorf("converting field CInfo: %v", err)
 	}
 	if err := x.BInfo.toC(&xc.b_info); err != nil {
-		return err
+		return fmt.Errorf("converting field BInfo: %v", err)
 	}
 	if numDisks := len(x.Disks); numDisks > 0 {
 		xc.disks = (*C.libxl_device_disk)(C.malloc(C.ulong(numDisks) * C.sizeof_libxl_device_disk))
@@ -2615,7 +2615,7 @@ func (x *DomainConfig) toC(xc *C.libxl_domain_config) (err error) {
 		cDisks := (*[1 << 28]C.libxl_device_disk)(unsafe.Pointer(xc.disks))[:numDisks:numDisks]
 		for i, v := range x.Disks {
 			if err := v.toC(&cDisks[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Disks: %v", err)
 			}
 		}
 	}
@@ -2625,7 +2625,7 @@ func (x *DomainConfig) toC(xc *C.libxl_domain_config) (err error) {
 		cNics := (*[1 << 28]C.libxl_device_nic)(unsafe.Pointer(xc.nics))[:numNics:numNics]
 		for i, v := range x.Nics {
 			if err := v.toC(&cNics[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Nics: %v", err)
 			}
 		}
 	}
@@ -2635,7 +2635,7 @@ func (x *DomainConfig) toC(xc *C.libxl_domain_config) (err error) {
 		cPcidevs := (*[1 << 28]C.libxl_device_pci)(unsafe.Pointer(xc.pcidevs))[:numPcidevs:numPcidevs]
 		for i, v := range x.Pcidevs {
 			if err := v.toC(&cPcidevs[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Pcidevs: %v", err)
 			}
 		}
 	}
@@ -2645,7 +2645,7 @@ func (x *DomainConfig) toC(xc *C.libxl_domain_config) (err error) {
 		cRdms := (*[1 << 28]C.libxl_device_rdm)(unsafe.Pointer(xc.rdms))[:numRdms:numRdms]
 		for i, v := range x.Rdms {
 			if err := v.toC(&cRdms[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Rdms: %v", err)
 			}
 		}
 	}
@@ -2655,7 +2655,7 @@ func (x *DomainConfig) toC(xc *C.libxl_domain_config) (err error) {
 		cDtdevs := (*[1 << 28]C.libxl_device_dtdev)(unsafe.Pointer(xc.dtdevs))[:numDtdevs:numDtdevs]
 		for i, v := range x.Dtdevs {
 			if err := v.toC(&cDtdevs[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Dtdevs: %v", err)
 			}
 		}
 	}
@@ -2665,7 +2665,7 @@ func (x *DomainConfig) toC(xc *C.libxl_domain_config) (err error) {
 		cVfbs := (*[1 << 28]C.libxl_device_vfb)(unsafe.Pointer(xc.vfbs))[:numVfbs:numVfbs]
 		for i, v := range x.Vfbs {
 			if err := v.toC(&cVfbs[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Vfbs: %v", err)
 			}
 		}
 	}
@@ -2675,7 +2675,7 @@ func (x *DomainConfig) toC(xc *C.libxl_domain_config) (err error) {
 		cVkbs := (*[1 << 28]C.libxl_device_vkb)(unsafe.Pointer(xc.vkbs))[:numVkbs:numVkbs]
 		for i, v := range x.Vkbs {
 			if err := v.toC(&cVkbs[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Vkbs: %v", err)
 			}
 		}
 	}
@@ -2685,7 +2685,7 @@ func (x *DomainConfig) toC(xc *C.libxl_domain_config) (err error) {
 		cVtpms := (*[1 << 28]C.libxl_device_vtpm)(unsafe.Pointer(xc.vtpms))[:numVtpms:numVtpms]
 		for i, v := range x.Vtpms {
 			if err := v.toC(&cVtpms[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Vtpms: %v", err)
 			}
 		}
 	}
@@ -2695,7 +2695,7 @@ func (x *DomainConfig) toC(xc *C.libxl_domain_config) (err error) {
 		cP9S := (*[1 << 28]C.libxl_device_p9)(unsafe.Pointer(xc.p9s))[:numP9S:numP9S]
 		for i, v := range x.P9S {
 			if err := v.toC(&cP9S[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field P9S: %v", err)
 			}
 		}
 	}
@@ -2705,7 +2705,7 @@ func (x *DomainConfig) toC(xc *C.libxl_domain_config) (err error) {
 		cPvcallsifs := (*[1 << 28]C.libxl_device_pvcallsif)(unsafe.Pointer(xc.pvcallsifs))[:numPvcallsifs:numPvcallsifs]
 		for i, v := range x.Pvcallsifs {
 			if err := v.toC(&cPvcallsifs[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Pvcallsifs: %v", err)
 			}
 		}
 	}
@@ -2715,7 +2715,7 @@ func (x *DomainConfig) toC(xc *C.libxl_domain_config) (err error) {
 		cVdispls := (*[1 << 28]C.libxl_device_vdispl)(unsafe.Pointer(xc.vdispls))[:numVdispls:numVdispls]
 		for i, v := range x.Vdispls {
 			if err := v.toC(&cVdispls[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Vdispls: %v", err)
 			}
 		}
 	}
@@ -2725,7 +2725,7 @@ func (x *DomainConfig) toC(xc *C.libxl_domain_config) (err error) {
 		cVsnds := (*[1 << 28]C.libxl_device_vsnd)(unsafe.Pointer(xc.vsnds))[:numVsnds:numVsnds]
 		for i, v := range x.Vsnds {
 			if err := v.toC(&cVsnds[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Vsnds: %v", err)
 			}
 		}
 	}
@@ -2735,7 +2735,7 @@ func (x *DomainConfig) toC(xc *C.libxl_domain_config) (err error) {
 		cChannels := (*[1 << 28]C.libxl_device_channel)(unsafe.Pointer(xc.channels))[:numChannels:numChannels]
 		for i, v := range x.Channels {
 			if err := v.toC(&cChannels[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Channels: %v", err)
 			}
 		}
 	}
@@ -2745,7 +2745,7 @@ func (x *DomainConfig) toC(xc *C.libxl_domain_config) (err error) {
 		cUsbctrls := (*[1 << 28]C.libxl_device_usbctrl)(unsafe.Pointer(xc.usbctrls))[:numUsbctrls:numUsbctrls]
 		for i, v := range x.Usbctrls {
 			if err := v.toC(&cUsbctrls[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Usbctrls: %v", err)
 			}
 		}
 	}
@@ -2755,7 +2755,7 @@ func (x *DomainConfig) toC(xc *C.libxl_domain_config) (err error) {
 		cUsbdevs := (*[1 << 28]C.libxl_device_usbdev)(unsafe.Pointer(xc.usbdevs))[:numUsbdevs:numUsbdevs]
 		for i, v := range x.Usbdevs {
 			if err := v.toC(&cUsbdevs[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Usbdevs: %v", err)
 			}
 		}
 	}
@@ -2852,7 +2852,7 @@ func (x *Vtpminfo) fromC(xc *C.libxl_vtpminfo) error {
 	x.Evtch = int(xc.evtch)
 	x.Rref = int(xc.rref)
 	if err := x.Uuid.fromC(&xc.uuid); err != nil {
-		return err
+		return fmt.Errorf("converting field Uuid: %v", err)
 	}
 
 	return nil
@@ -2878,7 +2878,7 @@ func (x *Vtpminfo) toC(xc *C.libxl_vtpminfo) (err error) {
 	xc.evtch = C.int(x.Evtch)
 	xc.rref = C.int(x.Rref)
 	if err := x.Uuid.toC(&xc.uuid); err != nil {
-		return err
+		return fmt.Errorf("converting field Uuid: %v", err)
 	}
 
 	return nil
@@ -2936,10 +2936,10 @@ func (x *Vcpuinfo) fromC(xc *C.libxl_vcpuinfo) error {
 	x.Running = bool(xc.running)
 	x.VcpuTime = uint64(xc.vcpu_time)
 	if err := x.Cpumap.fromC(&xc.cpumap); err != nil {
-		return err
+		return fmt.Errorf("converting field Cpumap: %v", err)
 	}
 	if err := x.CpumapSoft.fromC(&xc.cpumap_soft); err != nil {
-		return err
+		return fmt.Errorf("converting field CpumapSoft: %v", err)
 	}
 
 	return nil
@@ -2959,10 +2959,10 @@ func (x *Vcpuinfo) toC(xc *C.libxl_vcpuinfo) (err error) {
 	xc.running = C.bool(x.Running)
 	xc.vcpu_time = C.uint64_t(x.VcpuTime)
 	if err := x.Cpumap.toC(&xc.cpumap); err != nil {
-		return err
+		return fmt.Errorf("converting field Cpumap: %v", err)
 	}
 	if err := x.CpumapSoft.toC(&xc.cpumap_soft); err != nil {
-		return err
+		return fmt.Errorf("converting field CpumapSoft: %v", err)
 	}
 
 	return nil
@@ -2983,7 +2983,7 @@ func (x *Physinfo) fromC(xc *C.libxl_physinfo) error {
 	x.MaxPossibleMfn = uint64(xc.max_possible_mfn)
 	x.NrNodes = uint32(xc.nr_nodes)
 	if err := x.HwCap.fromC(&xc.hw_cap); err != nil {
-		return err
+		return fmt.Errorf("converting field HwCap: %v", err)
 	}
 	x.CapHvm = bool(xc.cap_hvm)
 	x.CapPv = bool(xc.cap_pv)
@@ -3016,7 +3016,7 @@ func (x *Physinfo) toC(xc *C.libxl_physinfo) (err error) {
 	xc.max_possible_mfn = C.uint64_t(x.MaxPossibleMfn)
 	xc.nr_nodes = C.uint32_t(x.NrNodes)
 	if err := x.HwCap.toC(&xc.hw_cap); err != nil {
-		return err
+		return fmt.Errorf("converting field HwCap: %v", err)
 	}
 	xc.cap_hvm = C.bool(x.CapHvm)
 	xc.cap_pv = C.bool(x.CapPv)
@@ -3074,7 +3074,7 @@ func (x *Vdisplinfo) fromC(xc *C.libxl_vdisplinfo) error {
 		x.Connectors = make([]Connectorinfo, numConnectors)
 		for i, v := range cConnectors {
 			if err := x.Connectors[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Connectors: %v", err)
 			}
 		}
 	}
@@ -3106,7 +3106,7 @@ func (x *Vdisplinfo) toC(xc *C.libxl_vdisplinfo) (err error) {
 		cConnectors := (*[1 << 28]C.libxl_connectorinfo)(unsafe.Pointer(xc.connectors))[:numConnectors:numConnectors]
 		for i, v := range x.Connectors {
 			if err := v.toC(&cConnectors[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Connectors: %v", err)
 			}
 		}
 	}
@@ -3141,7 +3141,7 @@ func (x *Pcminfo) fromC(xc *C.libxl_pcminfo) error {
 		x.Streams = make([]Streaminfo, numVsndStreams)
 		for i, v := range cStreams {
 			if err := x.Streams[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Streams: %v", err)
 			}
 		}
 	}
@@ -3162,7 +3162,7 @@ func (x *Pcminfo) toC(xc *C.libxl_pcminfo) (err error) {
 		cStreams := (*[1 << 28]C.libxl_streaminfo)(unsafe.Pointer(xc.streams))[:numVsndStreams:numVsndStreams]
 		for i, v := range x.Streams {
 			if err := v.toC(&cStreams[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Streams: %v", err)
 			}
 		}
 	}
@@ -3183,7 +3183,7 @@ func (x *Vsndinfo) fromC(xc *C.libxl_vsndinfo) error {
 		x.Pcms = make([]Pcminfo, numVsndPcms)
 		for i, v := range cPcms {
 			if err := x.Pcms[i].fromC(&v); err != nil {
-				return err
+				return fmt.Errorf("converting field Pcms: %v", err)
 			}
 		}
 	}
@@ -3214,7 +3214,7 @@ func (x *Vsndinfo) toC(xc *C.libxl_vsndinfo) (err error) {
 		cPcms := (*[1 << 28]C.libxl_pcminfo)(unsafe.Pointer(xc.pcms))[:numVsndPcms:numVsndPcms]
 		for i, v := range x.Pcms {
 			if err := v.toC(&cPcms[i]); err != nil {
-				return err
+				return fmt.Errorf("converting field Pcms: %v", err)
 			}
 		}
 	}
@@ -3371,26 +3371,26 @@ func (x *SchedCredit2Params) toC(xc *C.libxl_sched_credit2_params) (err error) {
 func (x *DomainRemusInfo) fromC(xc *C.libxl_domain_remus_info) error {
 	x.Interval = int(xc.interval)
 	if err := x.AllowUnsafe.fromC(&xc.allow_unsafe); err != nil {
-		return err
+		return fmt.Errorf("converting field AllowUnsafe: %v", err)
 	}
 	if err := x.Blackhole.fromC(&xc.blackhole); err != nil {
-		return err
+		return fmt.Errorf("converting field Blackhole: %v", err)
 	}
 	if err := x.Compression.fromC(&xc.compression); err != nil {
-		return err
+		return fmt.Errorf("converting field Compression: %v", err)
 	}
 	if err := x.Netbuf.fromC(&xc.netbuf); err != nil {
-		return err
+		return fmt.Errorf("converting field Netbuf: %v", err)
 	}
 	x.Netbufscript = C.GoString(xc.netbufscript)
 	if err := x.Diskbuf.fromC(&xc.diskbuf); err != nil {
-		return err
+		return fmt.Errorf("converting field Diskbuf: %v", err)
 	}
 	if err := x.Colo.fromC(&xc.colo); err != nil {
-		return err
+		return fmt.Errorf("converting field Colo: %v", err)
 	}
 	if err := x.UserspaceColoProxy.fromC(&xc.userspace_colo_proxy); err != nil {
-		return err
+		return fmt.Errorf("converting field UserspaceColoProxy: %v", err)
 	}
 
 	return nil
@@ -3405,28 +3405,28 @@ func (x *DomainRemusInfo) toC(xc *C.libxl_domain_remus_info) (err error) {
 
 	xc.interval = C.int(x.Interval)
 	if err := x.AllowUnsafe.toC(&xc.allow_unsafe); err != nil {
-		return err
+		return fmt.Errorf("converting field AllowUnsafe: %v", err)
 	}
 	if err := x.Blackhole.toC(&xc.blackhole); err != nil {
-		return err
+		return fmt.Errorf("converting field Blackhole: %v", err)
 	}
 	if err := x.Compression.toC(&xc.compression); err != nil {
-		return err
+		return fmt.Errorf("converting field Compression: %v", err)
 	}
 	if err := x.Netbuf.toC(&xc.netbuf); err != nil {
-		return err
+		return fmt.Errorf("converting field Netbuf: %v", err)
 	}
 	if x.Netbufscript != "" {
 		xc.netbufscript = C.CString(x.Netbufscript)
 	}
 	if err := x.Diskbuf.toC(&xc.diskbuf); err != nil {
-		return err
+		return fmt.Errorf("converting field Diskbuf: %v", err)
 	}
 	if err := x.Colo.toC(&xc.colo); err != nil {
-		return err
+		return fmt.Errorf("converting field Colo: %v", err)
 	}
 	if err := x.UserspaceColoProxy.toC(&xc.userspace_colo_proxy); err != nil {
-		return err
+		return fmt.Errorf("converting field UserspaceColoProxy: %v", err)
 	}
 
 	return nil
@@ -3434,33 +3434,33 @@ func (x *DomainRemusInfo) toC(xc *C.libxl_domain_remus_info) (err error) {
 
 func (x *Event) fromC(xc *C.libxl_event) error {
 	if err := x.Link.fromC(&xc.link); err != nil {
-		return err
+		return fmt.Errorf("converting field Link: %v", err)
 	}
 	x.Domid = Domid(xc.domid)
 	if err := x.Domuuid.fromC(&xc.domuuid); err != nil {
-		return err
+		return fmt.Errorf("converting field Domuuid: %v", err)
 	}
 	x.ForUser = uint64(xc.for_user)
 	x.Type = EventType(xc._type)
 	switch x.Type {
-	case EventTypeOperationComplete:
-		var typeOperationComplete EventTypeUnionOperationComplete
-		if err := typeOperationComplete.fromC(xc); err != nil {
-			return err
-		}
-		x.TypeUnion = typeOperationComplete
 	case EventTypeDomainShutdown:
 		var typeDomainShutdown EventTypeUnionDomainShutdown
 		if err := typeDomainShutdown.fromC(xc); err != nil {
-			return err
+			return fmt.Errorf("converting field typeDomainShutdown: %v", err)
 		}
 		x.TypeUnion = typeDomainShutdown
 	case EventTypeDiskEject:
 		var typeDiskEject EventTypeUnionDiskEject
 		if err := typeDiskEject.fromC(xc); err != nil {
-			return err
+			return fmt.Errorf("converting field typeDiskEject: %v", err)
 		}
 		x.TypeUnion = typeDiskEject
+	case EventTypeOperationComplete:
+		var typeOperationComplete EventTypeUnionOperationComplete
+		if err := typeOperationComplete.fromC(xc); err != nil {
+			return fmt.Errorf("converting field typeOperationComplete: %v", err)
+		}
+		x.TypeUnion = typeOperationComplete
 	default:
 		return fmt.Errorf("invalid union key '%v'", x.Type)
 	}
@@ -3486,7 +3486,7 @@ func (x *EventTypeUnionDiskEject) fromC(xc *C.libxl_event) error {
 	tmp := (*C.libxl_event_type_union_disk_eject)(unsafe.Pointer(&xc.u[0]))
 	x.Vdev = C.GoString(tmp.vdev)
 	if err := x.Disk.fromC(&tmp.disk); err != nil {
-		return err
+		return fmt.Errorf("converting field Disk: %v", err)
 	}
 	return nil
 }
@@ -3509,11 +3509,11 @@ func (x *Event) toC(xc *C.libxl_event) (err error) {
 	}()
 
 	if err := x.Link.toC(&xc.link); err != nil {
-		return err
+		return fmt.Errorf("converting field Link: %v", err)
 	}
 	xc.domid = C.libxl_domid(x.Domid)
 	if err := x.Domuuid.toC(&xc.domuuid); err != nil {
-		return err
+		return fmt.Errorf("converting field Domuuid: %v", err)
 	}
 	xc.for_user = C.uint64_t(x.ForUser)
 	xc._type = C.libxl_event_type(x.Type)
@@ -3537,7 +3537,7 @@ func (x *Event) toC(xc *C.libxl_event) (err error) {
 			disk_eject.vdev = C.CString(tmp.Vdev)
 		}
 		if err := tmp.Disk.toC(&disk_eject.disk); err != nil {
-			return err
+			return fmt.Errorf("converting field Disk: %v", err)
 		}
 		disk_ejectBytes := C.GoBytes(unsafe.Pointer(&disk_eject), C.sizeof_libxl_event_type_union_disk_eject)
 		copy(xc.u[:], disk_ejectBytes)
@@ -3585,18 +3585,18 @@ func (x *PsrHwInfo) fromC(xc *C.libxl_psr_hw_info) error {
 	x.Id = uint32(xc.id)
 	x.Type = PsrFeatType(xc._type)
 	switch x.Type {
-	case PsrFeatTypeMba:
-		var typeMba PsrHwInfoTypeUnionMba
-		if err := typeMba.fromC(xc); err != nil {
-			return err
-		}
-		x.TypeUnion = typeMba
 	case PsrFeatTypeCat:
 		var typeCat PsrHwInfoTypeUnionCat
 		if err := typeCat.fromC(xc); err != nil {
-			return err
+			return fmt.Errorf("converting field typeCat: %v", err)
 		}
 		x.TypeUnion = typeCat
+	case PsrFeatTypeMba:
+		var typeMba PsrHwInfoTypeUnionMba
+		if err := typeMba.fromC(xc); err != nil {
+			return fmt.Errorf("converting field typeMba: %v", err)
+		}
+		x.TypeUnion = typeMba
 	default:
 		return fmt.Errorf("invalid union key '%v'", x.Type)
 	}
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Xen-devel] [PATCH v3 4/8] golang/xenlight: Errors are negative
  2020-01-17 15:57 [Xen-devel] [PATCH v3 1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC George Dunlap
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 2/8] go/xenlight: Fix CpuidPoliclyList conversion George Dunlap
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 3/8] go/xenlight: More informative error messages George Dunlap
@ 2020-01-17 15:57 ` George Dunlap
  2020-01-20 23:40   ` Nick Rosbrook
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 5/8] golang/xenlight: Default loglevel to DEBUG until we get everything working George Dunlap
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: George Dunlap @ 2020-01-17 15:57 UTC (permalink / raw)
  To: xen-devel; +Cc: Nick Rosbrook, George Dunlap

Commit 871e51d2d4 changed the sign on the xenlight error types (making
the values negative, same as the C-generated constants), but failed to
flip the sign in the Error() string function.  The result is that
ErrorNonspecific.String() prints "libxl error: 1" rather than the
human-readable error message.

Get rid of the whole issue by making libxlErrors a map, and mapping
actual error values to string, falling back to printing the actual
value of the Error type if it's not present.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
v2:
- Convert libxlErrors to a map.

CC: Nick Rosbrook <rosbrookn@ainfosec.com>
---
 tools/golang/xenlight/xenlight.go | 62 +++++++++++++++----------------
 1 file changed, 30 insertions(+), 32 deletions(-)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index 1299981713..aa1e63a61a 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -36,42 +36,40 @@ import (
 	"unsafe"
 )
 
-var libxlErrors = [...]string{
-	-ErrorNonspecific:                  "Non-specific error",
-	-ErrorVersion:                      "Wrong version",
-	-ErrorFail:                         "Failed",
-	-ErrorNi:                           "Not Implemented",
-	-ErrorNomem:                        "No memory",
-	-ErrorInval:                        "Invalid argument",
-	-ErrorBadfail:                      "Bad Fail",
-	-ErrorGuestTimedout:                "Guest timed out",
-	-ErrorTimedout:                     "Timed out",
-	-ErrorNoparavirt:                   "No Paravirtualization",
-	-ErrorNotReady:                     "Not ready",
-	-ErrorOseventRegFail:               "OS event registration failed",
-	-ErrorBufferfull:                   "Buffer full",
-	-ErrorUnknownChild:                 "Unknown child",
-	-ErrorLockFail:                     "Lock failed",
-	-ErrorJsonConfigEmpty:              "JSON config empty",
-	-ErrorDeviceExists:                 "Device exists",
-	-ErrorCheckpointDevopsDoesNotMatch: "Checkpoint devops does not match",
-	-ErrorCheckpointDeviceNotSupported: "Checkpoint device not supported",
-	-ErrorVnumaConfigInvalid:           "VNUMA config invalid",
-	-ErrorDomainNotfound:               "Domain not found",
-	-ErrorAborted:                      "Aborted",
-	-ErrorNotfound:                     "Not found",
-	-ErrorDomainDestroyed:              "Domain destroyed",
-	-ErrorFeatureRemoved:               "Feature removed",
+var libxlErrors = map[Error]string{
+	ErrorNonspecific:                  "Non-specific error",
+	ErrorVersion:                      "Wrong version",
+	ErrorFail:                         "Failed",
+	ErrorNi:                           "Not Implemented",
+	ErrorNomem:                        "No memory",
+	ErrorInval:                        "Invalid argument",
+	ErrorBadfail:                      "Bad Fail",
+	ErrorGuestTimedout:                "Guest timed out",
+	ErrorTimedout:                     "Timed out",
+	ErrorNoparavirt:                   "No Paravirtualization",
+	ErrorNotReady:                     "Not ready",
+	ErrorOseventRegFail:               "OS event registration failed",
+	ErrorBufferfull:                   "Buffer full",
+	ErrorUnknownChild:                 "Unknown child",
+	ErrorLockFail:                     "Lock failed",
+	ErrorJsonConfigEmpty:              "JSON config empty",
+	ErrorDeviceExists:                 "Device exists",
+	ErrorCheckpointDevopsDoesNotMatch: "Checkpoint devops does not match",
+	ErrorCheckpointDeviceNotSupported: "Checkpoint device not supported",
+	ErrorVnumaConfigInvalid:           "VNUMA config invalid",
+	ErrorDomainNotfound:               "Domain not found",
+	ErrorAborted:                      "Aborted",
+	ErrorNotfound:                     "Not found",
+	ErrorDomainDestroyed:              "Domain destroyed",
+	ErrorFeatureRemoved:               "Feature removed",
 }
 
 func (e Error) Error() string {
-	if 0 < int(e) && int(e) < len(libxlErrors) {
-		s := libxlErrors[e]
-		if s != "" {
-			return s
-		}
+	if s, ok := libxlErrors[e]; ok {
+		return s
 	}
-	return fmt.Sprintf("libxl error: %d", -e)
+
+	return fmt.Sprintf("libxl error: %d", e)
 }
 
 // Context represents a libxl_ctx.
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Xen-devel] [PATCH v3 5/8] golang/xenlight: Default loglevel to DEBUG until we get everything working
  2020-01-17 15:57 [Xen-devel] [PATCH v3 1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC George Dunlap
                   ` (2 preceding siblings ...)
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 4/8] golang/xenlight: Errors are negative George Dunlap
@ 2020-01-17 15:57 ` George Dunlap
  2020-01-20 23:41   ` Nick Rosbrook
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 6/8] golang/xenlight: Don't leak memory on context open failure George Dunlap
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: George Dunlap @ 2020-01-17 15:57 UTC (permalink / raw)
  To: xen-devel; +Cc: Nick Rosbrook, George Dunlap

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---

The other option would be to expose the XTL logging levels and let the
caller set them somehow.

CC: Nick Rosbrook <rosbrookn@ainfosec.com>
---
 tools/golang/xenlight/xenlight.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index aa1e63a61a..0e71f6ca7d 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -82,7 +82,7 @@ type Context struct {
 func NewContext() (*Context, error) {
 	var ctx Context
 
-	ctx.logger = C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0)
+	ctx.logger = C.xtl_createlogger_stdiostream(C.stderr, C.XTL_DEBUG, 0)
 
 	ret := C.libxl_ctx_alloc(&ctx.ctx, C.LIBXL_VERSION, 0,
 		(*C.xentoollog_logger)(unsafe.Pointer(ctx.logger)))
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Xen-devel] [PATCH v3 6/8] golang/xenlight: Don't leak memory on context open failure
  2020-01-17 15:57 [Xen-devel] [PATCH v3 1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC George Dunlap
                   ` (3 preceding siblings ...)
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 5/8] golang/xenlight: Default loglevel to DEBUG until we get everything working George Dunlap
@ 2020-01-17 15:57 ` George Dunlap
  2020-01-20 23:43   ` Nick Rosbrook
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 7/8] golang/xenlight: Notify xenlight of SIGCHLD George Dunlap
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: George Dunlap @ 2020-01-17 15:57 UTC (permalink / raw)
  To: xen-devel; +Cc: Nick Rosbrook, George Dunlap

If libxl_ctx_alloc() returns an error, we need to destroy the logger
that we made.

Restructure the Close() method such that it checks for each resource
to be freed and then frees it.  This allows Close() to be come
idempotent, as well as to be a useful clean-up to a partially-created
context.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
CC: Nick Rosbrook <rosbrookn@ainfosec.com>
---
 tools/golang/xenlight/xenlight.go | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index 0e71f6ca7d..662b266250 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -79,28 +79,40 @@ type Context struct {
 }
 
 // NewContext returns a new Context.
-func NewContext() (*Context, error) {
-	var ctx Context
+func NewContext() (ctx *Context, err error) {
+	ctx = &Context{}
+
+	defer func() {
+		if err != nil {
+			ctx.Close()
+			ctx = nil
+		}
+	}()
 
 	ctx.logger = C.xtl_createlogger_stdiostream(C.stderr, C.XTL_DEBUG, 0)
 
 	ret := C.libxl_ctx_alloc(&ctx.ctx, C.LIBXL_VERSION, 0,
 		(*C.xentoollog_logger)(unsafe.Pointer(ctx.logger)))
 	if ret != 0 {
-		return nil, Error(ret)
+		return ctx, Error(ret)
 	}
 
-	return &ctx, nil
+	return ctx, nil
 }
 
 // Close closes the Context.
 func (ctx *Context) Close() error {
-	ret := C.libxl_ctx_free(ctx.ctx)
-	ctx.ctx = nil
-	C.xtl_logger_destroy((*C.xentoollog_logger)(unsafe.Pointer(ctx.logger)))
+	if ctx.ctx != nil {
+		ret := C.libxl_ctx_free(ctx.ctx)
+		if ret != 0 {
+			return Error(ret)
+		}
+		ctx.ctx = nil
+	}
 
-	if ret != 0 {
-		return Error(ret)
+	if ctx.logger != nil {
+		C.xtl_logger_destroy((*C.xentoollog_logger)(unsafe.Pointer(ctx.logger)))
+		ctx.logger = nil
 	}
 
 	return nil
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Xen-devel] [PATCH v3 7/8] golang/xenlight: Notify xenlight of SIGCHLD
  2020-01-17 15:57 [Xen-devel] [PATCH v3 1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC George Dunlap
                   ` (4 preceding siblings ...)
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 6/8] golang/xenlight: Don't leak memory on context open failure George Dunlap
@ 2020-01-17 15:57 ` George Dunlap
  2020-01-17 16:52   ` Ian Jackson
  2020-01-17 18:13   ` [Xen-devel] [PATCH v3 7/8] golang/xenlight: Notify xenlight of SIGCHLD Nick Rosbrook
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 8/8] RFC: Sketch constructors, DomainCreateNew George Dunlap
                   ` (2 subsequent siblings)
  8 siblings, 2 replies; 32+ messages in thread
From: George Dunlap @ 2020-01-17 15:57 UTC (permalink / raw)
  To: xen-devel; +Cc: Nick Rosbrook, Ian Jackson, George Dunlap

libxl forks external processes and waits for them to complete; it
therefore needs to be notified when children exit.

In absence of instructions to the contrary, libxl sets up its own
SIGCHLD handlers.

Golang always unmasks and handles SIGCHLD itself.  libxl thankfully
notices this and throws an assert() rather than clobbering SIGCHLD
handlers.

Tell libxl that we'll be responsible for getting SIGCHLD notifications
to it.  Arrange for a channel in the context to receive notifications
on SIGCHLD, and set up a goroutine that will pass these on to libxl.

NB that every libxl context needs a notification; so multiple contexts
will each spin up their own goroutine when opening a context, and shut
it down on close.

libxl also wants to hold on to a const pointer to
xenlight_childproc_hooks rather than do a copy; so make a global
structure in C space and initialize it once on library creation.

While here, add a few comments to make the context set-up a bit easier
to follow.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
v2:
- Fix unsafe libxl_childproc_hooks pointer behavior
- Close down the SIGCHLD handler first, and make sure it's exited
  before closing the context
- Explicitly decide to have a separate goroutine per ctx

NB that due to a bug in libxl, this will hang without Ian's "[PATCH v2
00/10] libxl: event: Fix hang for some applications" series.

CC: Nick Rosbrook <rosbrookn@ainfosec.com>
CC: Ian Jackson <ian.jackson@citrix.com>
---
 tools/golang/xenlight/xenlight.go | 72 ++++++++++++++++++++++++++++++-
 1 file changed, 70 insertions(+), 2 deletions(-)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index 662b266250..c462e4bb42 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -20,6 +20,8 @@ package xenlight
 #cgo LDFLAGS: -lxenlight -lyajl -lxentoollog
 #include <stdlib.h>
 #include <libxl.h>
+
+libxl_childproc_hooks xenlight_childproc_hooks;
 */
 import "C"
 
@@ -33,6 +35,9 @@ import "C"
 
 import (
 	"fmt"
+	"os"
+	"os/signal"
+	"syscall"
 	"unsafe"
 )
 
@@ -72,10 +77,49 @@ func (e Error) Error() string {
 	return fmt.Sprintf("libxl error: %d", e)
 }
 
+func init() {
+	// libxl for some reason wants to:
+	// 1. Retain a copy to this pointer as long as the context is open, and
+	// 2. Not free it when it's done
+	//
+	// Rather than alloc and free multiple copies, just keep a single
+	// static copy in the C space (since C code isn't allowed to retain pointers
+	// to go code), and initialize it once.
+	C.xenlight_childproc_hooks.chldowner = C.libxl_sigchld_owner_mainloop
+}
+
 // Context represents a libxl_ctx.
 type Context struct {
-	ctx    *C.libxl_ctx
-	logger *C.xentoollog_logger_stdiostream
+	ctx         *C.libxl_ctx
+	logger      *C.xentoollog_logger_stdiostream
+	sigchld     chan os.Signal
+	sigchldDone chan bool
+}
+
+// Golang always unmasks SIGCHLD, and internally has ways of
+// distributing SIGCHLD to multiple recipients.  libxl has provision
+// for this model: just tell it when a SIGCHLD happened, and it will
+// look after its own processes.
+//
+// This should "play nicely" with other users of SIGCHLD as long as
+// they don't reap libxl's processes.
+//
+// Every context needs to be notified on each SIGCHLD; so spin up a
+// new goroutine for each context.  If there are a large number of contexts,
+// this means each context will be woken up looking through its own list of children.
+//
+// The alternate would be to register a fork callback, such that the
+// xenlight package can make a single list of all children, and only
+// notify the specific libxl context(s) that have children woken.  But
+// it's not clear to me this will be much more work than having the
+// xenlight go library do the same thing; doing it in separate go
+// threads has the potential to do it in parallel.  Leave that as an
+// optimization for later if it turns out to be a bottleneck.
+func sigchldHandler(ctx *Context) {
+	for _ = range ctx.sigchld {
+		go C.libxl_childproc_sigchld_occurred(ctx.ctx)
+	}
+	close(ctx.sigchldDone)
 }
 
 // NewContext returns a new Context.
@@ -89,19 +133,43 @@ func NewContext() (ctx *Context, err error) {
 		}
 	}()
 
+	// Create a logger
 	ctx.logger = C.xtl_createlogger_stdiostream(C.stderr, C.XTL_DEBUG, 0)
 
+	// Allocate a context
 	ret := C.libxl_ctx_alloc(&ctx.ctx, C.LIBXL_VERSION, 0,
 		(*C.xentoollog_logger)(unsafe.Pointer(ctx.logger)))
 	if ret != 0 {
 		return ctx, Error(ret)
 	}
 
+	// Tell libxl that we'll be dealing with SIGCHLD...
+	C.libxl_childproc_setmode(ctx.ctx, &C.xenlight_childproc_hooks, nil)
+
+	// ...and arrange to keep that promise.
+	ctx.sigchld = make(chan os.Signal, 2)
+	ctx.sigchldDone = make(chan bool, 1)
+	signal.Notify(ctx.sigchld, syscall.SIGCHLD)
+
+	go sigchldHandler(ctx)
+
 	return ctx, nil
 }
 
 // Close closes the Context.
 func (ctx *Context) Close() error {
+	// Tell our SIGCHLD notifier to shut down, and wait for it to exit
+	// before we free the context.
+	if ctx.sigchld == nil {
+		signal.Stop(ctx.sigchld)
+		close(ctx.sigchld)
+
+		<-ctx.sigchldDone
+
+		ctx.sigchld = nil
+		ctx.sigchldDone = nil
+	}
+
 	if ctx.ctx != nil {
 		ret := C.libxl_ctx_free(ctx.ctx)
 		if ret != 0 {
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Xen-devel] [PATCH v3 8/8] RFC: Sketch constructors, DomainCreateNew
  2020-01-17 15:57 [Xen-devel] [PATCH v3 1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC George Dunlap
                   ` (5 preceding siblings ...)
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 7/8] golang/xenlight: Notify xenlight of SIGCHLD George Dunlap
@ 2020-01-17 15:57 ` George Dunlap
  2020-01-17 18:38   ` George Dunlap
                     ` (2 more replies)
  2020-01-17 16:04 ` [Xen-devel] [PATCH v3 1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC George Dunlap
  2020-01-20 23:39 ` Nick Rosbrook
  8 siblings, 3 replies; 32+ messages in thread
From: George Dunlap @ 2020-01-17 15:57 UTC (permalink / raw)
  To: xen-devel; +Cc: Nick Rosbrook, George Dunlap

This is a sketch of functionality suitable for creating a basic
domain, with a disk and a vif.  DomainConfig, DeviceDisk, and
DeviceNic types are all created using constructor functions, which
initialize them with libxl's defaults.

DomainCreateNew takes the config and calls without any updates.

Obviously some of these will need to be changed it we switch to
passing references to .toC() rather than passing back by value.

The main purpose of this is to allow testing of creating a hard-coded
domain.

Creating a domain would look like this:

	// type = "pv"
	dconf, err := xl.NewDomainConfig(xl.DomainTypePv)
	if err != nil {
		fmt.Printf("NewDomainConfig: %v\n", err)
		return
	}
	dconf.CInfo.Type = xl.DomainTypePv
	// name = "c6-01"
	dconf.CInfo.Name = "c6-01"
	// vcpus=4
	dconf.BInfo.MaxVcpus = 4
	// memory = "2048"
	dconf.BInfo.MaxMemkb = 2048 * 1024
	dconf.BInfo.TargetMemkb = 2048 * 1024
	// on_crash = 'destroy'
	dconf.OnCrash = xl.ActionOnShutdownDestroy
	// bootloader = "pygrub"
	dconf.BInfo.Bootloader = "pygrub"
	// disk = [ 'vdev=hda,format=raw,target=/images/c6-01.raw']
	{
		disk, err := xl.NewDeviceDisk()
		if err != nil {
			fmt.Printf("NewDeviceDisk: %v\n", err)
			return
		}
		disk.Vdev = "hda"
		//disk.DiskBackend = xl.DiskBackendPhy
		disk.Format = xl.DiskFormatRaw
		disk.Readwrite = 1
		disk.PdevPath = "/images/c6-01.raw"
		dconf.Disks = append(dconf.Disks, *disk)
	}
	// vif = [ 'mac=5a:5b:d6:f1:d6:b4' ]
	{
		vif, err := xl.NewDeviceNic()
		if err != nil {
			fmt.Printf("NewDeviceNic: %v\n", err)
			return
		}
		vif.Mac = xl.Mac{ 0x5a, 0x5b, 0xd6, 0x31, 0xd6, 0xb4 }
		dconf.Nics = append(dconf.Nics, *vif)
	}
	// serial='pty' # HVM only

	did, err := ctx.DomainCreateNew(dconf)

	if err != nil {
		fmt.Printf("Creating domain: %v\n", err)
		return
	}

	fmt.Printf("Domain %s(%d) created successfully", dconf.CInfo.Name, did)


Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
CC: Nick Rosbrook <rosbrookn@ainfosec.com>
---
 tools/golang/xenlight/xenlight.go | 66 +++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index c462e4bb42..5a21a2b9b8 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -1113,3 +1113,69 @@ func (Ctx *Context) PrimaryConsoleGetTty(domid uint32) (path string, err error)
 	path = C.GoString(cpath)
 	return
 }
+
+func NewDomainConfig(t DomainType) (*DomainConfig, error) {
+	var cconfig C.libxl_domain_config
+
+	C.libxl_domain_config_init(&cconfig)
+	C.libxl_domain_build_info_init_type(&cconfig.b_info, C.libxl_domain_type(t))
+
+	gconfig := &DomainConfig{}
+	err := gconfig.fromC(&cconfig)
+	if err != nil {
+		return nil, err
+	}
+
+	return gconfig, nil
+}
+
+func NewDeviceDisk() (*DeviceDisk, error) {
+	var ctype C.libxl_device_disk
+
+	C.libxl_device_disk_init(&ctype)
+
+	gtype := &DeviceDisk{}
+	err := gtype.fromC(&ctype)
+
+	if err != nil {
+		return nil, err
+	}
+
+	return gtype, nil
+}
+
+func NewDeviceNic() (*DeviceNic, error) {
+	var ctype C.libxl_device_nic
+
+	C.libxl_device_nic_init(&ctype)
+
+	gtype := &DeviceNic{}
+	err := gtype.fromC(&ctype)
+
+	if err != nil {
+		return nil, err
+	}
+
+	return gtype, nil
+}
+
+// int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config,
+//                             uint32_t *domid,
+//                             const libxl_asyncop_how *ao_how,
+//                             const libxl_asyncprogress_how *aop_console_how)
+func (Ctx *Context) DomainCreateNew(config *DomainConfig) (Domid, error) {
+	var cdomid C.uint32_t
+	var cconfig C.libxl_domain_config
+	err := config.toC(&cconfig)
+	if err != nil {
+		return Domid(0), fmt.Errorf("converting domain config to C: %v", err)
+	}
+	defer C.libxl_domain_config_dispose(&cconfig)
+
+	ret := C.libxl_domain_create_new(Ctx.ctx, &cconfig, &cdomid, nil, nil)
+	if ret != 0 {
+		return Domid(0), Error(ret)
+	}
+
+	return Domid(cdomid), nil
+}
-- 
2.24.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC
  2020-01-17 15:57 [Xen-devel] [PATCH v3 1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC George Dunlap
                   ` (6 preceding siblings ...)
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 8/8] RFC: Sketch constructors, DomainCreateNew George Dunlap
@ 2020-01-17 16:04 ` George Dunlap
  2020-01-20 23:39 ` Nick Rosbrook
  8 siblings, 0 replies; 32+ messages in thread
From: George Dunlap @ 2020-01-17 16:04 UTC (permalink / raw)
  To: xen-devel; +Cc: Nick Rosbrook

On 1/17/20 3:57 PM, George Dunlap wrote:
> The current fromC array code will do the "magic" casting and
> martialling even when num_foo variable is 0.  Go crashes when doing
> the cast.
> 
> Only do array marshalling if the number of elements is non-zero;
> otherwise, leave the target pointer empty (nil for Go slices, NULL for
> C arrays).
> 
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
> ---
> v2:
> - Remove toC part of this, which has been folded into Nick's patch
>   series.

Er, obviously the subject line should say "v2", for the whole series. :-/

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 7/8] golang/xenlight: Notify xenlight of SIGCHLD
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 7/8] golang/xenlight: Notify xenlight of SIGCHLD George Dunlap
@ 2020-01-17 16:52   ` Ian Jackson
  2020-01-17 17:33     ` George Dunlap
  2020-01-17 18:13   ` [Xen-devel] [PATCH v3 7/8] golang/xenlight: Notify xenlight of SIGCHLD Nick Rosbrook
  1 sibling, 1 reply; 32+ messages in thread
From: Ian Jackson @ 2020-01-17 16:52 UTC (permalink / raw)
  To: George Dunlap; +Cc: Nick Rosbrook, xen-devel, Ian Jackson

George Dunlap writes ("[PATCH v3 7/8] golang/xenlight: Notify xenlight of SIGCHLD"):
> libxl forks external processes and waits for them to complete; it
> therefore needs to be notified when children exit.
> 
> In absence of instructions to the contrary, libxl sets up its own
> SIGCHLD handlers.
> 
> Golang always unmasks and handles SIGCHLD itself.  libxl thankfully
> notices this and throws an assert() rather than clobbering SIGCHLD
> handlers.
> 
> Tell libxl that we'll be responsible for getting SIGCHLD notifications
> to it.  Arrange for a channel in the context to receive notifications
> on SIGCHLD, and set up a goroutine that will pass these on to libxl.
> 
> NB that every libxl context needs a notification; so multiple contexts
> will each spin up their own goroutine when opening a context, and shut
> it down on close.
> 
> libxl also wants to hold on to a const pointer to
> xenlight_childproc_hooks rather than do a copy; so make a global
> structure in C space and initialize it once on library creation.
> 
> While here, add a few comments to make the context set-up a bit easier
> to follow.

For what it's worth,

Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>

However, I don't think I understand golang (and particularly the
threading model etc.) well enough for that to mean I'm confident that
this correct.

> +func init() {
> +	// libxl for some reason wants to:
> +	// 1. Retain a copy to this pointer as long as the context is open, and
> +	// 2. Not free it when it's done

I found this comment a bit rude.  This is not an unusual approach for
a pointer in a C API.

In Rust this would be called an `immutable borrow': the ctx borrows
the contents of the pointer, promises not to modify it (and the caller
ought to promise not to modify it either); but the caller retains
ownership so when the ctx is done the borrow ends.

Normally in C the struct would be `static const', so there is no need
to allocate it or free it.

I think that ...

> +	// Rather than alloc and free multiple copies, just keep a single
> +	// static copy in the C space (since C code isn't allowed to retain pointers
> +	// to go code), and initialize it once.
> +	C.xenlight_childproc_hooks.chldowner = C.libxl_sigchld_owner_mainloop

... this is what this is doing ?

> +// This should "play nicely" with other users of SIGCHLD as long as
> +// they don't reap libxl's processes.

I assume that nothing in golang will do this.  If something calls a
non-process-specific variant of wait* then you would need to somehow
capture the results and feed them to libxl_childproc_exited.

> +// The alternate would be to register a fork callback, such that the
> +// xenlight package can make a single list of all children, and only
> +// notify the specific libxl context(s) that have children woken.  But
> +// it's not clear to me this will be much more work than having the
> +// xenlight go library do the same thing; doing it in separate go
> +// threads has the potential to do it in parallel.  Leave that as an
> +// optimization for later if it turns out to be a bottleneck.

I think this is fine.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 7/8] golang/xenlight: Notify xenlight of SIGCHLD
  2020-01-17 16:52   ` Ian Jackson
@ 2020-01-17 17:33     ` George Dunlap
  2020-01-17 18:12       ` [Xen-devel] [PATCH] libxl: event: Document lifetime API for libxl_childproc_setmode Ian Jackson
  0 siblings, 1 reply; 32+ messages in thread
From: George Dunlap @ 2020-01-17 17:33 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Nick Rosbrook, xen-devel

On 1/17/20 4:52 PM, Ian Jackson wrote:
> George Dunlap writes ("[PATCH v3 7/8] golang/xenlight: Notify xenlight of SIGCHLD"):
>> libxl forks external processes and waits for them to complete; it
>> therefore needs to be notified when children exit.
>>
>> In absence of instructions to the contrary, libxl sets up its own
>> SIGCHLD handlers.
>>
>> Golang always unmasks and handles SIGCHLD itself.  libxl thankfully
>> notices this and throws an assert() rather than clobbering SIGCHLD
>> handlers.
>>
>> Tell libxl that we'll be responsible for getting SIGCHLD notifications
>> to it.  Arrange for a channel in the context to receive notifications
>> on SIGCHLD, and set up a goroutine that will pass these on to libxl.
>>
>> NB that every libxl context needs a notification; so multiple contexts
>> will each spin up their own goroutine when opening a context, and shut
>> it down on close.
>>
>> libxl also wants to hold on to a const pointer to
>> xenlight_childproc_hooks rather than do a copy; so make a global
>> structure in C space and initialize it once on library creation.
>>
>> While here, add a few comments to make the context set-up a bit easier
>> to follow.
> 
> For what it's worth,
> 
> Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> However, I don't think I understand golang (and particularly the
> threading model etc.) well enough for that to mean I'm confident that
> this correct.

Thanks -- I mainly just wanted to give you the opportunity to spot any
obvious things I was doing wrong wrt libxl.  (For instance, an earlier
version of this patch had me destroying the libxl context before
shutting down the sigchld helper, which is obviously wrong.)

>> +func init() {
>> +	// libxl for some reason wants to:
>> +	// 1. Retain a copy to this pointer as long as the context is open, and
>> +	// 2. Not free it when it's done
> 
> I found this comment a bit rude.  This is not an unusual approach for
> a pointer in a C API.>
> In Rust this would be called an `immutable borrow': the ctx borrows
> the contents of the pointer, promises not to modify it (and the caller
> ought to promise not to modify it either); but the caller retains
> ownership so when the ctx is done the borrow ends.

I'm sorry to be rude; I've deleted the comment.  But none of what you
said is obvious from the documentation; on the contrary:

---
...is equivalent to &{ libxl_sigchld_owner_libxl, 0, 0 }
---

...seems to imply that you can pass it a pointer to the stack.  And,
from an interface perspective, that seems obviously better to me --
rather than make the caller promise not to change the contents (to
who-knows-what result if they forget), it's much easier to just take a
local copy and update it with locks next time the function is called.

I was slightly more annoyed because Go's rule about C functions not
retaining pointers to Go memory meant I had to do some un-Golike things
to make this work; but that's nothing to do with libxl.

> Normally in C the struct would be `static const', so there is no need
> to allocate it or free it.
> 
> I think that ...
> 
>> +	// Rather than alloc and free multiple copies, just keep a single
>> +	// static copy in the C space (since C code isn't allowed to retain pointers
>> +	// to go code), and initialize it once.
>> +	C.xenlight_childproc_hooks.chldowner = C.libxl_sigchld_owner_mainloop
> 
> ... this is what this is doing ?

In fact, there's a global C variable declared here:

---
 #include <libxl.h>
+
+libxl_childproc_hooks xenlight_childproc_hooks;
 */
 import "C"
---

...and the line above just initialized it.  But on reflection I've
decided to do this:

---
/*

#cgo LDFLAGS: -lxenlight -lyajl -lxentoollog
#include <stdlib.h>
#include <libxl.h>

static const libxl_childproc_hooks childproc_hooks = { .chldowner =
libxl_sigchld_owner_mainloop };

void xenlight_set_chldproc(libxl_ctx *ctx) {
	libxl_childproc_setmode(ctx, &childproc_hooks, NULL);
}

*/
import "C"
---

That declares childproc_hooks as static const in the C space; and then
defines a C function which takes a libxl_ctx* and calls
libxl_childproc_setmode appropriately.  That way childproc_hooks can
enjoy the safety of static const.

>> +// The alternate would be to register a fork callback, such that the
>> +// xenlight package can make a single list of all children, and only
>> +// notify the specific libxl context(s) that have children woken.  But
>> +// it's not clear to me this will be much more work than having the
>> +// xenlight go library do the same thing; doing it in separate go
>> +// threads has the potential to do it in parallel.  Leave that as an
>> +// optimization for later if it turns out to be a bottleneck.
> 
> I think this is fine.

Thanks.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [Xen-devel] [PATCH] libxl: event: Document lifetime API for libxl_childproc_setmode
  2020-01-17 17:33     ` George Dunlap
@ 2020-01-17 18:12       ` Ian Jackson
  2020-01-20 12:06         ` Wei Liu
  0 siblings, 1 reply; 32+ messages in thread
From: Ian Jackson @ 2020-01-17 18:12 UTC (permalink / raw)
  To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, George Dunlap, Wei Liu

There is already an identical comment for
libxl_osevent_register_hooks.

libxl_childproc_setmode's hooks parameter has the same property and
this should be documented.

Reported-by; George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxl/libxl_event.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/libxl/libxl_event.h b/tools/libxl/libxl_event.h
index d1517f7456..8d0aa6417e 100644
--- a/tools/libxl/libxl_event.h
+++ b/tools/libxl/libxl_event.h
@@ -548,6 +548,8 @@ typedef struct {
  * May not be called when libxl might have any child processes, or the
  * behaviour is undefined.  So it is best to call this at
  * initialisation.
+ *
+ * The value *hooks is not copied and must outlast the libxl_ctx.
  */
 void libxl_childproc_setmode(libxl_ctx *ctx, const libxl_childproc_hooks *hooks,
                              void *user);
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 7/8] golang/xenlight: Notify xenlight of SIGCHLD
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 7/8] golang/xenlight: Notify xenlight of SIGCHLD George Dunlap
  2020-01-17 16:52   ` Ian Jackson
@ 2020-01-17 18:13   ` Nick Rosbrook
  2020-01-17 18:28     ` George Dunlap
  1 sibling, 1 reply; 32+ messages in thread
From: Nick Rosbrook @ 2020-01-17 18:13 UTC (permalink / raw)
  To: George Dunlap; +Cc: Nick Rosbrook, Xen-devel, Ian Jackson

>  // Context represents a libxl_ctx.
>  type Context struct {
> -       ctx    *C.libxl_ctx
> -       logger *C.xentoollog_logger_stdiostream
> +       ctx         *C.libxl_ctx
> +       logger      *C.xentoollog_logger_stdiostream
> +       sigchld     chan os.Signal
> +       sigchldDone chan bool

It's preferred to use `chan struct{}` for this pattern; it makes it
clear that the data sent over the channel has no meaning, and is only
intended to be used for synchronization.

> +       // ...and arrange to keep that promise.
> +       ctx.sigchld = make(chan os.Signal, 2)
> +       ctx.sigchldDone = make(chan bool, 1)
> +       signal.Notify(ctx.sigchld, syscall.SIGCHLD)
> +
> +       go sigchldHandler(ctx)

It could be useful to add a comment here that explains the lifetime of
this goroutine, i.e. it returns when the context is Close()'d.

>  // Close closes the Context.
>  func (ctx *Context) Close() error {
> +       // Tell our SIGCHLD notifier to shut down, and wait for it to exit
> +       // before we free the context.
> +       if ctx.sigchld == nil {

Shouldn't this be `if ctx.sigchld != nil`?

-NR

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 7/8] golang/xenlight: Notify xenlight of SIGCHLD
  2020-01-17 18:13   ` [Xen-devel] [PATCH v3 7/8] golang/xenlight: Notify xenlight of SIGCHLD Nick Rosbrook
@ 2020-01-17 18:28     ` George Dunlap
  0 siblings, 0 replies; 32+ messages in thread
From: George Dunlap @ 2020-01-17 18:28 UTC (permalink / raw)
  To: Nick Rosbrook; +Cc: Nick Rosbrook, Xen-devel, Ian Jackson

On 1/17/20 6:13 PM, Nick Rosbrook wrote:
>>  // Context represents a libxl_ctx.
>>  type Context struct {
>> -       ctx    *C.libxl_ctx
>> -       logger *C.xentoollog_logger_stdiostream
>> +       ctx         *C.libxl_ctx
>> +       logger      *C.xentoollog_logger_stdiostream
>> +       sigchld     chan os.Signal
>> +       sigchldDone chan bool
> 
> It's preferred to use `chan struct{}` for this pattern; it makes it
> clear that the data sent over the channel has no meaning, and is only
> intended to be used for synchronization.

OK.  I think it looks ugly, but there's certainly a signalling value to
having it really be empty.

> 
>> +       // ...and arrange to keep that promise.
>> +       ctx.sigchld = make(chan os.Signal, 2)
>> +       ctx.sigchldDone = make(chan bool, 1)
>> +       signal.Notify(ctx.sigchld, syscall.SIGCHLD)
>> +
>> +       go sigchldHandler(ctx)
> 
> It could be useful to add a comment here that explains the lifetime of
> this goroutine, i.e. it returns when the context is Close()'d.

Ack.

>>  // Close closes the Context.
>>  func (ctx *Context) Close() error {
>> +       // Tell our SIGCHLD notifier to shut down, and wait for it to exit
>> +       // before we free the context.
>> +       if ctx.sigchld == nil {
> 
> Shouldn't this be `if ctx.sigchld != nil`?

Er, yes, indeed it should.  This has gone through too many iterations...

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 8/8] RFC: Sketch constructors, DomainCreateNew
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 8/8] RFC: Sketch constructors, DomainCreateNew George Dunlap
@ 2020-01-17 18:38   ` George Dunlap
  2020-01-22 10:32   ` George Dunlap
  2020-01-24 19:32   ` Nick Rosbrook
  2 siblings, 0 replies; 32+ messages in thread
From: George Dunlap @ 2020-01-17 18:38 UTC (permalink / raw)
  To: xen-devel; +Cc: Nick Rosbrook

On 1/17/20 3:57 PM, George Dunlap wrote:
> This is a sketch of functionality suitable for creating a basic
> domain, with a disk and a vif.  DomainConfig, DeviceDisk, and
> DeviceNic types are all created using constructor functions, which
> initialize them with libxl's defaults.
> 
> DomainCreateNew takes the config and calls without any updates.
> 
> Obviously some of these will need to be changed it we switch to
> passing references to .toC() rather than passing back by value.
> 
> The main purpose of this is to allow testing of creating a hard-coded
> domain.
> 
> Creating a domain would look like this:
> 
> 	// type = "pv"
> 	dconf, err := xl.NewDomainConfig(xl.DomainTypePv)
> 	if err != nil {
> 		fmt.Printf("NewDomainConfig: %v\n", err)
> 		return
> 	}
> 	dconf.CInfo.Type = xl.DomainTypePv
> 	// name = "c6-01"
> 	dconf.CInfo.Name = "c6-01"
> 	// vcpus=4
> 	dconf.BInfo.MaxVcpus = 4
> 	// memory = "2048"
> 	dconf.BInfo.MaxMemkb = 2048 * 1024
> 	dconf.BInfo.TargetMemkb = 2048 * 1024
> 	// on_crash = 'destroy'
> 	dconf.OnCrash = xl.ActionOnShutdownDestroy
> 	// bootloader = "pygrub"
> 	dconf.BInfo.Bootloader = "pygrub"
> 	// disk = [ 'vdev=hda,format=raw,target=/images/c6-01.raw']
> 	{
> 		disk, err := xl.NewDeviceDisk()
> 		if err != nil {
> 			fmt.Printf("NewDeviceDisk: %v\n", err)
> 			return
> 		}
> 		disk.Vdev = "hda"
> 		//disk.DiskBackend = xl.DiskBackendPhy
> 		disk.Format = xl.DiskFormatRaw
> 		disk.Readwrite = 1
> 		disk.PdevPath = "/images/c6-01.raw"
> 		dconf.Disks = append(dconf.Disks, *disk)
> 	}
> 	// vif = [ 'mac=5a:5b:d6:f1:d6:b4' ]
> 	{
> 		vif, err := xl.NewDeviceNic()
> 		if err != nil {
> 			fmt.Printf("NewDeviceNic: %v\n", err)
> 			return
> 		}
> 		vif.Mac = xl.Mac{ 0x5a, 0x5b, 0xd6, 0x31, 0xd6, 0xb4 }
> 		dconf.Nics = append(dconf.Nics, *vif)
> 	}
> 	// serial='pty' # HVM only
> 
> 	did, err := ctx.DomainCreateNew(dconf)
> 
> 	if err != nil {
> 		fmt.Printf("Creating domain: %v\n", err)
> 		return
> 	}
> 
> 	fmt.Printf("Domain %s(%d) created successfully", dconf.CInfo.Name, did)
> 
> 
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
> ---
> CC: Nick Rosbrook <rosbrookn@ainfosec.com>
> ---
>  tools/golang/xenlight/xenlight.go | 66 +++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> index c462e4bb42..5a21a2b9b8 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -1113,3 +1113,69 @@ func (Ctx *Context) PrimaryConsoleGetTty(domid uint32) (path string, err error)
>  	path = C.GoString(cpath)
>  	return
>  }
> +
> +func NewDomainConfig(t DomainType) (*DomainConfig, error) {
> +	var cconfig C.libxl_domain_config
> +
> +	C.libxl_domain_config_init(&cconfig)
> +	C.libxl_domain_build_info_init_type(&cconfig.b_info, C.libxl_domain_type(t))
> +
> +	gconfig := &DomainConfig{}
> +	err := gconfig.fromC(&cconfig)
> +	if err != nil {
> +		return nil, err
> +	}
> +
> +	return gconfig, nil
> +}
> +
> +func NewDeviceDisk() (*DeviceDisk, error) {
> +	var ctype C.libxl_device_disk
> +
> +	C.libxl_device_disk_init(&ctype)
> +
> +	gtype := &DeviceDisk{}
> +	err := gtype.fromC(&ctype)
> +
> +	if err != nil {
> +		return nil, err
> +	}
> +
> +	return gtype, nil
> +}
> +
> +func NewDeviceNic() (*DeviceNic, error) {
> +	var ctype C.libxl_device_nic
> +
> +	C.libxl_device_nic_init(&ctype)
> +
> +	gtype := &DeviceNic{}
> +	err := gtype.fromC(&ctype)
> +
> +	if err != nil {
> +		return nil, err
> +	}
> +
> +	return gtype, nil
> +}
> +
> +// int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config,
> +//                             uint32_t *domid,
> +//                             const libxl_asyncop_how *ao_how,
> +//                             const libxl_asyncprogress_how *aop_console_how)
> +func (Ctx *Context) DomainCreateNew(config *DomainConfig) (Domid, error) {
> +	var cdomid C.uint32_t
> +	var cconfig C.libxl_domain_config
> +	err := config.toC(&cconfig)
> +	if err != nil {
> +		return Domid(0), fmt.Errorf("converting domain config to C: %v", err)
> +	}
> +	defer C.libxl_domain_config_dispose(&cconfig)
> +
> +	ret := C.libxl_domain_create_new(Ctx.ctx, &cconfig, &cdomid, nil, nil)
> +	if ret != 0 {
> +		return Domid(0), Error(ret)
> +	}
> +
> +	return Domid(cdomid), nil
> +}

An alternate way to do this would be something like this:

---
func NewDomainConfig(t DomainType, populate func(*DomainConfig))
*DomainConfig {
	var ctype C.libxl_domain_config

	C.libxl_domain_config_init(&ctype)
	C.libxl_domain_build_info_init_type(&ctype.b_info, C.libxl_domain_type(t))

	gtype := &DomainConfig{}
	err := gtype.fromC(&ctype)
	if err != nil {
		panic("internal error: Can't convert empty DomainConfig")
	}

	if populate != nil {
		populate(gtype)
	}

	return gtype
}

func NewDeviceDisk(populate func(*DeviceDisk)) *DeviceDisk {
	var ctype C.libxl_device_disk

	C.libxl_device_disk_init(&ctype)

	gtype := &DeviceDisk{}
	err := gtype.fromC(&ctype)

	if err != nil {
		panic("internal error: Can't convert empty DeviceDisk")
	}

	if populate != nil {
		populate(gtype)
	}

	return gtype
}

func NewDeviceNic(populate func(*DeviceNic)) *DeviceNic {
	var ctype C.libxl_device_nic

	C.libxl_device_nic_init(&ctype)

	gtype := &DeviceNic{}
	err := gtype.fromC(&ctype)

	if err != nil {
		panic("internal error: Can't convert empty DeviceNic")
	}

	if populate != nil {
		populate(gtype)
	}

	return gtype
}
---

And then code to populate a domain config might look like this:

	dconf := xl.NewDomainConfig(xl.DomainTypePv, func(dconf *xl.DomainConfig) {
		dconf.CInfo.Type = xl.DomainTypePv
		// name = "c6-01"
		dconf.CInfo.Name = "c6-01"
		// vcpus=4
		dconf.BInfo.MaxVcpus = 4
		// memory = "2048"
		dconf.BInfo.MaxMemkb = 2048 * 1024
		dconf.BInfo.TargetMemkb = 2048 * 1024
		// on_crash = 'destroy'
		dconf.OnCrash = xl.ActionOnShutdownDestroy
		// bootloader = "pygrub"
		dconf.BInfo.Bootloader = "pygrub"
	})

	dconf.Disks = []xl.DeviceDisk{*xl.NewDeviceDisk(func(disk *xl.DeviceDisk) {
		disk.Vdev = "hda"
		//disk.DiskBackend = xl.DiskBackendPhy
		disk.Format = xl.DiskFormatRaw
		disk.Readwrite = 1
		disk.PdevPath = "/images/c6-01.raw"
		dconf.Disks = append(dconf.Disks, *disk)
	})}

	dconf.Nics = []xl.DeviceNic{*xl.NewDeviceNic(func(vif *xl.DeviceNic) {
		vif.Mac = xl.Mac{0x5a, 0x5b, 0xd6, 0x31, 0xd6, 0xb4}
	})}


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH] libxl: event: Document lifetime API for libxl_childproc_setmode
  2020-01-17 18:12       ` [Xen-devel] [PATCH] libxl: event: Document lifetime API for libxl_childproc_setmode Ian Jackson
@ 2020-01-20 12:06         ` Wei Liu
  0 siblings, 0 replies; 32+ messages in thread
From: Wei Liu @ 2020-01-20 12:06 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Anthony PERARD, xen-devel, George Dunlap, Wei Liu

On Fri, Jan 17, 2020 at 06:12:07PM +0000, Ian Jackson wrote:
> There is already an identical comment for
> libxl_osevent_register_hooks.
> 
> libxl_childproc_setmode's hooks parameter has the same property and
> this should be documented.
> 
> Reported-by; George Dunlap <george.dunlap@citrix.com>
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Acked-by: Wei Liu <wl@xen.org>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 2/8] go/xenlight: Fix CpuidPoliclyList conversion
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 2/8] go/xenlight: Fix CpuidPoliclyList conversion George Dunlap
@ 2020-01-20 23:30   ` Nick Rosbrook
  0 siblings, 0 replies; 32+ messages in thread
From: Nick Rosbrook @ 2020-01-20 23:30 UTC (permalink / raw)
  To: George Dunlap; +Cc: Nick Rosbrook, Xen-devel

> Empty Go strings should be converted to `nil` libxl_cpuid_policy_list;
> otherwise libxl_cpuid_parse_config gets confused.
>
> Also, libxl_cpuid_policy_list returns a weird error, not a "normal"
> libxl error; if it returns one of these non-standard errors, convert
> it to ErrorInval.
>
> Finally, make the fromC() method take a pointer, and set the value of
> CpuidPolicyList such that it will generate a valid CpuidPolicyList in
> response.
>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Nick Rosbrook <rosbrookn@ainfosec.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 3/8] go/xenlight: More informative error messages
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 3/8] go/xenlight: More informative error messages George Dunlap
@ 2020-01-20 23:32   ` Nick Rosbrook
  0 siblings, 0 replies; 32+ messages in thread
From: Nick Rosbrook @ 2020-01-20 23:32 UTC (permalink / raw)
  To: George Dunlap; +Cc: Nick Rosbrook, Xen-devel

> If an error is encountered deep in a complicated data structure, it's
> often difficult to tell where the error actually is.  Make the error
> message from the generated toC() and fromC() structures more
> informative by tagging which field being converted encountered the
> error.  This will have the effect of giving a "stack trace" of the
> failure inside a nested data structure.
>
> NB that my version of python insists on reordering a couple of switch
> statements for some reason; In other patches I've reverted those
> changes, but in this case it's more difficult because they interact
> with actual code changes.  I'll leave this here for now, as we're
> going to remove helpers.gen.go from being tracked by git at some point
> in the near future anyway.
>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>

Reviewed-by: Nick Rosbrook <rosbrookn@ainfosec.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC
  2020-01-17 15:57 [Xen-devel] [PATCH v3 1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC George Dunlap
                   ` (7 preceding siblings ...)
  2020-01-17 16:04 ` [Xen-devel] [PATCH v3 1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC George Dunlap
@ 2020-01-20 23:39 ` Nick Rosbrook
  2020-01-21 17:35   ` George Dunlap
  8 siblings, 1 reply; 32+ messages in thread
From: Nick Rosbrook @ 2020-01-20 23:39 UTC (permalink / raw)
  To: George Dunlap; +Cc: Nick Rosbrook, Xen-devel

 Sorry I didn't catch this the first time around, but:

> diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
> index b9a7e828a0..889807d928 100644
> --- a/tools/golang/xenlight/helpers.gen.go
> +++ b/tools/golang/xenlight/helpers.gen.go
> @@ -623,12 +623,14 @@ func (x *SchedParams) toC(xc *C.libxl_sched_params) (err error) {
>
>  func (x *VcpuSchedParams) fromC(xc *C.libxl_vcpu_sched_params) error {
>         x.Sched = Scheduler(xc.sched)
> -       numVcpus := int(xc.num_vcpus)
> -       cVcpus := (*[1 << 28]C.libxl_sched_params)(unsafe.Pointer(xc.vcpus))[:numVcpus:numVcpus]
> -       x.Vcpus = make([]SchedParams, numVcpus)
> -       for i, v := range cVcpus {
> -               if err := x.Vcpus[i].fromC(&v); err != nil {
> -                       return err
> +       x.Vcpus = nil
> +       if numVcpus := int(xc.num_vcpus); numVcpus > 0 {


Since `numX` is now scoped to this if block, we could probably just
use `n` or similar and then drop `golenvar` from
`xenlight_golang_array_fromC`. It would remove some pretty ugly
variable names from the generated code :)

-NR

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 4/8] golang/xenlight: Errors are negative
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 4/8] golang/xenlight: Errors are negative George Dunlap
@ 2020-01-20 23:40   ` Nick Rosbrook
  0 siblings, 0 replies; 32+ messages in thread
From: Nick Rosbrook @ 2020-01-20 23:40 UTC (permalink / raw)
  To: George Dunlap; +Cc: Nick Rosbrook, Xen-devel

> Commit 871e51d2d4 changed the sign on the xenlight error types (making
> the values negative, same as the C-generated constants), but failed to
> flip the sign in the Error() string function.  The result is that
> ErrorNonspecific.String() prints "libxl error: 1" rather than the
> human-readable error message.
>
> Get rid of the whole issue by making libxlErrors a map, and mapping
> actual error values to string, falling back to printing the actual
> value of the Error type if it's not present.
>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Nick Rosbrook <rosbrookn@ainfosec.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 5/8] golang/xenlight: Default loglevel to DEBUG until we get everything working
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 5/8] golang/xenlight: Default loglevel to DEBUG until we get everything working George Dunlap
@ 2020-01-20 23:41   ` Nick Rosbrook
  2020-01-21  9:55     ` George Dunlap
  0 siblings, 1 reply; 32+ messages in thread
From: Nick Rosbrook @ 2020-01-20 23:41 UTC (permalink / raw)
  To: George Dunlap; +Cc: Nick Rosbrook, Xen-devel

> The other option would be to expose the XTL logging levels and let the
> caller set them somehow.
I think this is fine for now.

For the future, I like using the "functional option" pattern for this
sort of thing. That way, if a user wanted to set a non-default log
level, they could do:

ctx, err := xenlight.NewContext(xenlight.WithLogLevel(lvl))

but if they do not need to specify any options, it's still just:

ctx, err := xenlight.NewContext()

-NR

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 6/8] golang/xenlight: Don't leak memory on context open failure
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 6/8] golang/xenlight: Don't leak memory on context open failure George Dunlap
@ 2020-01-20 23:43   ` Nick Rosbrook
  0 siblings, 0 replies; 32+ messages in thread
From: Nick Rosbrook @ 2020-01-20 23:43 UTC (permalink / raw)
  To: George Dunlap; +Cc: Nick Rosbrook, Xen-devel

> If libxl_ctx_alloc() returns an error, we need to destroy the logger
> that we made.
>
> Restructure the Close() method such that it checks for each resource
> to be freed and then frees it.  This allows Close() to be come
> idempotent, as well as to be a useful clean-up to a partially-created
> context.
>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Nick Rosbrook <rosbrookn@ainfosec.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 5/8] golang/xenlight: Default loglevel to DEBUG until we get everything working
  2020-01-20 23:41   ` Nick Rosbrook
@ 2020-01-21  9:55     ` George Dunlap
  2020-01-24 19:51       ` Nick Rosbrook
  0 siblings, 1 reply; 32+ messages in thread
From: George Dunlap @ 2020-01-21  9:55 UTC (permalink / raw)
  To: Nick Rosbrook; +Cc: Nick Rosbrook, Xen-devel



> On Jan 20, 2020, at 11:41 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
> 
>> The other option would be to expose the XTL logging levels and let the
>> caller set them somehow.
> I think this is fine for now.
> 
> For the future, I like using the "functional option" pattern for this
> sort of thing. That way, if a user wanted to set a non-default log
> level, they could do:
> 
> ctx, err := xenlight.NewContext(xenlight.WithLogLevel(lvl))
> 
> but if they do not need to specify any options, it's still just:
> 
> ctx, err := xenlight.NewContext()

You know, I somehow remembered the “use a function to set options” pattern (and have a  mock-up for that in the “NewType()” patch later), but didn’t notice that such function were variadic.  That’s a lot nicer.

But really, we need a way to actually create a logger properly.  Apparently one thing libvirt does is to create a logger to a file for each guest.  That’s something our package users  might want to do at some point.

 -George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC
  2020-01-20 23:39 ` Nick Rosbrook
@ 2020-01-21 17:35   ` George Dunlap
  0 siblings, 0 replies; 32+ messages in thread
From: George Dunlap @ 2020-01-21 17:35 UTC (permalink / raw)
  To: Nick Rosbrook; +Cc: Nick Rosbrook, Xen-devel

On 1/20/20 11:39 PM, Nick Rosbrook wrote:
>  Sorry I didn't catch this the first time around, but:
> 
>> diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
>> index b9a7e828a0..889807d928 100644
>> --- a/tools/golang/xenlight/helpers.gen.go
>> +++ b/tools/golang/xenlight/helpers.gen.go
>> @@ -623,12 +623,14 @@ func (x *SchedParams) toC(xc *C.libxl_sched_params) (err error) {
>>
>>  func (x *VcpuSchedParams) fromC(xc *C.libxl_vcpu_sched_params) error {
>>         x.Sched = Scheduler(xc.sched)
>> -       numVcpus := int(xc.num_vcpus)
>> -       cVcpus := (*[1 << 28]C.libxl_sched_params)(unsafe.Pointer(xc.vcpus))[:numVcpus:numVcpus]
>> -       x.Vcpus = make([]SchedParams, numVcpus)
>> -       for i, v := range cVcpus {
>> -               if err := x.Vcpus[i].fromC(&v); err != nil {
>> -                       return err
>> +       x.Vcpus = nil
>> +       if numVcpus := int(xc.num_vcpus); numVcpus > 0 {
> 
> 
> Since `numX` is now scoped to this if block, we could probably just
> use `n` or similar and then drop `golenvar` from
> `xenlight_golang_array_fromC`. It would remove some pretty ugly
> variable names from the generated code :)

Yes, that looks good.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 8/8] RFC: Sketch constructors, DomainCreateNew
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 8/8] RFC: Sketch constructors, DomainCreateNew George Dunlap
  2020-01-17 18:38   ` George Dunlap
@ 2020-01-22 10:32   ` George Dunlap
  2020-01-24 19:32   ` Nick Rosbrook
  2 siblings, 0 replies; 32+ messages in thread
From: George Dunlap @ 2020-01-22 10:32 UTC (permalink / raw)
  To: xen-devel; +Cc: Nick Rosbrook

On 1/17/20 3:57 PM, George Dunlap wrote:
> This is a sketch of functionality suitable for creating a basic
> domain, with a disk and a vif.  DomainConfig, DeviceDisk, and
> DeviceNic types are all created using constructor functions, which
> initialize them with libxl's defaults.
> 
> DomainCreateNew takes the config and calls without any updates.
> 
> Obviously some of these will need to be changed it we switch to
> passing references to .toC() rather than passing back by value.
> 
> The main purpose of this is to allow testing of creating a hard-coded
> domain.

BTW, just in case anyone decides to try this -- this will create a
domain in the "paused" state, and it does not create a "reaper" for it
when the domain shuts down.  You have to manually `xl unpause` the
domain for it to run, and then `xl destroy` the domain after it shuts down.

The first is just an implementation detail; the second requires adding
the ability to receive events, which will be the next "level" of libxl
event handling support.

 -George


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 8/8] RFC: Sketch constructors, DomainCreateNew
  2020-01-17 15:57 ` [Xen-devel] [PATCH v3 8/8] RFC: Sketch constructors, DomainCreateNew George Dunlap
  2020-01-17 18:38   ` George Dunlap
  2020-01-22 10:32   ` George Dunlap
@ 2020-01-24 19:32   ` Nick Rosbrook
  2020-01-27 18:08     ` George Dunlap
  2 siblings, 1 reply; 32+ messages in thread
From: Nick Rosbrook @ 2020-01-24 19:32 UTC (permalink / raw)
  To: George Dunlap; +Cc: Nick Rosbrook, Xen-devel

Sorry for the late reply on this one.

> +func NewDomainConfig(t DomainType) (*DomainConfig, error) {
> +       var cconfig C.libxl_domain_config
> +
> +       C.libxl_domain_config_init(&cconfig)
> +       C.libxl_domain_build_info_init_type(&cconfig.b_info, C.libxl_domain_type(t))
> +
> +       gconfig := &DomainConfig{}
> +       err := gconfig.fromC(&cconfig)
> +       if err != nil {
> +               return nil, err
> +       }
> +
> +       return gconfig, nil
> +}

I think this is sufficient for the "New" functions; simple is probably
better here. I appreciate the idea of the `populate func` approach you
mentioned in another email, but I think in practice people will want
to leverage encoding/json or otherwise to unmarshal some data into a
DomainConfig etc. Or, we would hopefully be able to unmarshal an
xl.cfg. All that is to say, I think the approach you have shown here
gives us and package users the most flexibility.

Do you have any thoughts on supporting unmarshaling json, xl.cfg, etc.?

If we stick with this outline for constructors, they will be easy to
generate. I'm happy to work on that, unless you were already planning
on it.

Thanks,
-NR

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 5/8] golang/xenlight: Default loglevel to DEBUG until we get everything working
  2020-01-21  9:55     ` George Dunlap
@ 2020-01-24 19:51       ` Nick Rosbrook
  0 siblings, 0 replies; 32+ messages in thread
From: Nick Rosbrook @ 2020-01-24 19:51 UTC (permalink / raw)
  To: George Dunlap; +Cc: Nick Rosbrook, Xen-devel

> But really, we need a way to actually create a logger properly.  Apparently one thing libvirt does is to create a logger to a file for each guest.  That’s something our package users  might want to do at some point.

Yes, I think package users will want logging to be pretty flexible. I
think we can cover most of those cases with Context options, or maybe
we make a Logger type that provides an abstraction for XTL.

-NR

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 8/8] RFC: Sketch constructors, DomainCreateNew
  2020-01-24 19:32   ` Nick Rosbrook
@ 2020-01-27 18:08     ` George Dunlap
  2020-01-28 20:41       ` Nick Rosbrook
  0 siblings, 1 reply; 32+ messages in thread
From: George Dunlap @ 2020-01-27 18:08 UTC (permalink / raw)
  To: Nick Rosbrook; +Cc: Nick Rosbrook, Xen-devel

On 1/24/20 7:32 PM, Nick Rosbrook wrote:
> Sorry for the late reply on this one.
> 
>> +func NewDomainConfig(t DomainType) (*DomainConfig, error) {
>> +       var cconfig C.libxl_domain_config
>> +
>> +       C.libxl_domain_config_init(&cconfig)
>> +       C.libxl_domain_build_info_init_type(&cconfig.b_info, C.libxl_domain_type(t))
>> +
>> +       gconfig := &DomainConfig{}
>> +       err := gconfig.fromC(&cconfig)
>> +       if err != nil {
>> +               return nil, err
>> +       }
>> +
>> +       return gconfig, nil
>> +}
> 
> I think this is sufficient for the "New" functions; simple is probably
> better here. I appreciate the idea of the `populate func` approach you
> mentioned in another email, but I think in practice people will want
> to leverage encoding/json or otherwise to unmarshal some data into a
> DomainConfig etc. Or, we would hopefully be able to unmarshal an
> xl.cfg. All that is to say, I think the approach you have shown here
> gives us and package users the most flexibility.

I think marshaling and unmarshalling should be fine, *as long as* the
structure being unmarshalled actually went through the
libxl_<type>_init() function first.

In fact, I was kicking around the idea of adding a non-exported field to
all the generated structs -- maybe "bool initalized" -- and having the
.fromC() methods set this to 'true', and the .toC() methods return an
error if it wasn't set.  But then we'd need to write custom JSON
marshallers to handle these.

I'm not personally very interested in parsing xl.cfg files, but libxl
has library functions to do that, so it should be something very easy to
add if you want.  (Although in fact, it looks like a lot of the code to
actually interpret the results of the parsing is in xl; we might want to
see about moving some of that functionality into libxlu.)

> If we stick with this outline for constructors, they will be easy to
> generate. I'm happy to work on that, unless you were already planning
> on it.

I'm afraid my 1 day a week of coding is going to have to be diverted to
something else for a month or so; so please go ahead if you have the time.

As far as further steps -- do you have a clear idea what kind of
functionality you'd like to see possible by the time of the feature
freeze (probably in May)?  Do you have plans to use these bindings
yourself, and if so, how?

For my part, I want to start and reap guests.  The latter will require
adding event callback functionality which will require more thought (and
perhaps expose more libxl issues).  But I don't yet have a clear target
beyond that.

Re function calls -- do we want to just manually duplicate them as
needed, or try to get some sort of IDL support?

Other work items include:

* modifying configure to detect the availability of go and enable the
bindings if it's available

* Enabling go build testing in the gitlab CI loop

* Making `go get` work, which might involve having code to push
post-generated code to a repo and tagging as appropriate

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 8/8] RFC: Sketch constructors, DomainCreateNew
  2020-01-27 18:08     ` George Dunlap
@ 2020-01-28 20:41       ` Nick Rosbrook
  2020-01-29 14:17         ` Nick Rosbrook
  2020-01-29 14:46         ` George Dunlap
  0 siblings, 2 replies; 32+ messages in thread
From: Nick Rosbrook @ 2020-01-28 20:41 UTC (permalink / raw)
  To: George Dunlap; +Cc: Nick Rosbrook, Xen-devel

> I think marshaling and unmarshalling should be fine, *as long as* the
> structure being unmarshalled actually went through the
> libxl_<type>_init() function first.
>
> In fact, I was kicking around the idea of adding a non-exported field to
> all the generated structs -- maybe "bool initalized" -- and having the
> .fromC() methods set this to 'true', and the .toC() methods return an
> error if it wasn't set.  But then we'd need to write custom JSON
> marshallers to handle these.

I *think* to guarantee that libxl_<type>_init() has been called when
unmarshaling, we would need to generate UnmarshalJSON functions to
implement json.Unmarshaler. E.g.,:

func (dd *DeviceDisk) UnmarshalJSON(data []byte) error {
        if dd == nil { // Or maybe this is the 'initialized' check you mentioned
                dd, err := NewDeviceDisk()
                if err != nil {
                        return err
                }
        }

        return json.Unmarshal(data, dd)
}

AFAICT, this would be required for someone to unmarshal a complete
domain configuration in one go.

> I'm not personally very interested in parsing xl.cfg files, but libxl
> has library functions to do that, so it should be something very easy to
> add if you want.  (Although in fact, it looks like a lot of the code to
> actually interpret the results of the parsing is in xl; we might want to
> see about moving some of that functionality into libxlu.)

Okay, noted.

> > If we stick with this outline for constructors, they will be easy to
> > generate. I'm happy to work on that, unless you were already planning
> > on it.
>
> I'm afraid my 1 day a week of coding is going to have to be diverted to
> something else for a month or so; so please go ahead if you have the time.

Okay, I think I can manage this fairly easily.

> As far as further steps -- do you have a clear idea what kind of
> functionality you'd like to see possible by the time of the feature
> freeze (probably in May)?  Do you have plans to use these bindings
> yourself, and if so, how?
>
> For my part, I want to start and reap guests.  The latter will require
> adding event callback functionality which will require more thought (and
> perhaps expose more libxl issues).  But I don't yet have a clear target
> beyond that.

Yes, I plan on using these bindings in redctl (our Redfield toolstack)
[1], to replace our os/exec calls to xl. To fully make that
transition, we would need domain start/stop, PCI and network
attach/detach, as well as some utilities (most of which are either
implemented, or would be easy to do). But, making that transition is
relatively low on the priority list right now, so I can't commit to a
timeline unfortunately.

> Re function calls -- do we want to just manually duplicate them as
> needed, or try to get some sort of IDL support?

I think it will make more sense to manually duplicate them as needed.
That way, we can be more particular about function signatures etc. to
ensure they are idiomatic Go. Also, I am of the opinion that a minimal
API is a better place to start. Which brings me to another question
and potential work item:

Do we want to re-evaluate what is currently implemented in the API? Do
you have plans to use everything that is currently there? If not, it
might be nice to trim off things we don't need right now.

> Other work items include:
>
> * modifying configure to detect the availability of go and enable the
> bindings if it's available
>
> * Enabling go build testing in the gitlab CI loop
>
> * Making `go get` work, which might involve having code to push
> post-generated code to a repo and tagging as appropriate

I was going to ask about this. You had a vanity URL in place at one
point, right? Did `go get` ever work with that? In any case, pushing
to another repo might be desirable.

Thanks,
-NR

[1] https://gitlab.com/redfield/redctl

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 8/8] RFC: Sketch constructors, DomainCreateNew
  2020-01-28 20:41       ` Nick Rosbrook
@ 2020-01-29 14:17         ` Nick Rosbrook
  2020-01-29 14:46         ` George Dunlap
  1 sibling, 0 replies; 32+ messages in thread
From: Nick Rosbrook @ 2020-01-29 14:17 UTC (permalink / raw)
  To: George Dunlap; +Cc: Nick Rosbrook, Xen-devel

> I *think* to guarantee that libxl_<type>_init() has been called when
> unmarshaling, we would need to generate UnmarshalJSON functions to
> implement json.Unmarshaler. E.g.,:
>
> func (dd *DeviceDisk) UnmarshalJSON(data []byte) error {
>         if dd == nil { // Or maybe this is the 'initialized' check you mentioned

Err, I mean `if dd == (DeviceDisk{})`. We want to check if the value
that dd points to is the DeviceDisk zero value.

-NR

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 8/8] RFC: Sketch constructors, DomainCreateNew
  2020-01-28 20:41       ` Nick Rosbrook
  2020-01-29 14:17         ` Nick Rosbrook
@ 2020-01-29 14:46         ` George Dunlap
  2020-02-04 19:26           ` Nick Rosbrook
  1 sibling, 1 reply; 32+ messages in thread
From: George Dunlap @ 2020-01-29 14:46 UTC (permalink / raw)
  To: Nick Rosbrook; +Cc: Nick Rosbrook, Xen-devel

On 1/28/20 8:41 PM, Nick Rosbrook wrote:
>> I think marshaling and unmarshalling should be fine, *as long as* the
>> structure being unmarshalled actually went through the
>> libxl_<type>_init() function first.
>>
>> In fact, I was kicking around the idea of adding a non-exported field to
>> all the generated structs -- maybe "bool initalized" -- and having the
>> .fromC() methods set this to 'true', and the .toC() methods return an
>> error if it wasn't set.  But then we'd need to write custom JSON
>> marshallers to handle these.
> 
> I *think* to guarantee that libxl_<type>_init() has been called when
> unmarshaling, we would need to generate UnmarshalJSON functions to
> implement json.Unmarshaler. E.g.,:
> 
> func (dd *DeviceDisk) UnmarshalJSON(data []byte) error {
>         if dd == nil { // Or maybe this is the 'initialized' check you mentioned
>                 dd, err := NewDeviceDisk()
>                 if err != nil {
>                         return err
>                 }
>         }
> 
>         return json.Unmarshal(data, dd)
> }
> 
> AFAICT, this would be required for someone to unmarshal a complete
> domain configuration in one go.

So the above will fix an issue like this:

Scenario A
1. Make a structure from version V by calling NewType()
2. Fill in what you want
3. Marshal it into json
4. Marshal it out of json into a structure from version V+1, with new fields

With code as above, the new elements of structure V+1 will be
initialized by the NewType() in the UnmarshalJSON() method.

But the problem I'm worried about is this:

Scenario B
1. Make an empty, uninitialized structure, without calling NewType()
2. Fill in some fields
3. Marshal it into json
4. Marshal it out of json (with the same version)

In the case above, step 3 encodes all the known fields with *golang*'s
zero values, rather than libxl's default values, and so step 4 will
clobber any defaults written by NewType() with golang zero values again.

Of course, something like scenario A might happen without the marshal /
unmarshal, which is why I thought having a private 'initialized' flag
might be helpful.  But then what you'd want to solve B by having the
unmarshaller read the initialized flag and add it to the json / read it
from the json (since the json package itself can't do it).

(Naturally people can directly modify the json to have the 'initialized'
flag set, but if you get to that level of messing around, you get to
keep all the pieces if it breaks.)

>> As far as further steps -- do you have a clear idea what kind of
>> functionality you'd like to see possible by the time of the feature
>> freeze (probably in May)?  Do you have plans to use these bindings
>> yourself, and if so, how?
>>
>> For my part, I want to start and reap guests.  The latter will require
>> adding event callback functionality which will require more thought (and
>> perhaps expose more libxl issues).  But I don't yet have a clear target
>> beyond that.
> 
> Yes, I plan on using these bindings in redctl (our Redfield toolstack)
> [1], to replace our os/exec calls to xl. To fully make that
> transition, we would need domain start/stop, PCI and network
> attach/detach, as well as some utilities (most of which are either
> implemented, or would be easy to do). But, making that transition is
> relatively low on the priority list right now, so I can't commit to a
> timeline unfortunately.

Sure, nor I; but having a goal always helps, even if it's only best-effort.

Looking at redctl, it seems like actually a pretty full-featured
toolstack -- that seems like a nice complete target to aim at. :-)

>> Re function calls -- do we want to just manually duplicate them as
>> needed, or try to get some sort of IDL support?
> 
> I think it will make more sense to manually duplicate them as needed.
> That way, we can be more particular about function signatures etc. to
> ensure they are idiomatic Go. Also, I am of the opinion that a minimal
> API is a better place to start. Which brings me to another question
> and potential work item:
> 
> Do we want to re-evaluate what is currently implemented in the API? Do
> you have plans to use everything that is currently there? If not, it
> might be nice to trim off things we don't need right now.

I think we should make sure that things actually work.  The very
original golang bindings I wrote to be able to control cpupools, so I
think those functions should stay.  But I'm not sure if anyone's ever
used ConsoleGetTty.  Like `xl.cfg` parsing, it's the sort of thing that
*somebody* will probably want eventually; so I'm inclined to say it
would be less cost to just test it and make sure it works than to remove
it and re-add it when someone decides they need it.

Did you have anything in particular in mind?

I was sort of thinking what we might do is leave `xenlight` as mostly
just a plain wrapper around the libxl C functions, as close to what
might be generated as possible; and then have another package that would
do something more useful.  For instance, having 'Vm' struct, which could
be Start()ed, shut down, and so on; and which would keep track of
if/when the domain died, &c.

>> Other work items include:
>>
>> * modifying configure to detect the availability of go and enable the
>> bindings if it's available
>>
>> * Enabling go build testing in the gitlab CI loop
>>
>> * Making `go get` work, which might involve having code to push
>> post-generated code to a repo and tagging as appropriate
> 
> I was going to ask about this. You had a vanity URL in place at one
> point, right? Did `go get` ever work with that? In any case, pushing
> to another repo might be desirable.
We never actually had a URL in place, no.  I had simply chosen the URL
based on what would be a good combination of easy to type/remember and
easy to set up effectively.  (This was after having an informal chat
with Ian Jackson, who tends to do most of this sort of technical admin
thing.)  We'd probably want to go back and figure out what kind of
"interface" is possible, how to do versioning &c, and then work
backwards from that to get a workflow from the various Xen branches into
that.

BTW do you guys have a solution to the "install new tools then reboot"
issue?  I guess if you have a daemon then it will retain the old version
of the library until after you reboot?

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Xen-devel] [PATCH v3 8/8] RFC: Sketch constructors, DomainCreateNew
  2020-01-29 14:46         ` George Dunlap
@ 2020-02-04 19:26           ` Nick Rosbrook
  0 siblings, 0 replies; 32+ messages in thread
From: Nick Rosbrook @ 2020-02-04 19:26 UTC (permalink / raw)
  To: George Dunlap; +Cc: Nick Rosbrook, Xen-devel

> But the problem I'm worried about is this:
>
> Scenario B
> 1. Make an empty, uninitialized structure, without calling NewType()
> 2. Fill in some fields
> 3. Marshal it into json
> 4. Marshal it out of json (with the same version)
>
> In the case above, step 3 encodes all the known fields with *golang*'s
> zero values, rather than libxl's default values, and so step 4 will
> clobber any defaults written by NewType() with golang zero values again.

One way to solve this would be add struct tags to generated types that
tell JSON to omit zero values when marshaling. I.e.,
`json:",omitempty"`. Then, only fields that were actually set will be
marshaled. And, for Unmarshal to overwrite a field set by NewType(),
it would have to be *explicitly* set by a user. But, that does mean if
a field is set, and happens to be a Go zero value, it will not be
shown in the JSON. That could be a problem itself.

> >> As far as further steps -- do you have a clear idea what kind of
> >> functionality you'd like to see possible by the time of the feature
> >> freeze (probably in May)?  Do you have plans to use these bindings
> >> yourself, and if so, how?
> >>
> >> For my part, I want to start and reap guests.  The latter will require
> >> adding event callback functionality which will require more thought (and
> >> perhaps expose more libxl issues).  But I don't yet have a clear target
> >> beyond that.
> >
> > Yes, I plan on using these bindings in redctl (our Redfield toolstack)
> > [1], to replace our os/exec calls to xl. To fully make that
> > transition, we would need domain start/stop, PCI and network
> > attach/detach, as well as some utilities (most of which are either
> > implemented, or would be easy to do). But, making that transition is
> > relatively low on the priority list right now, so I can't commit to a
> > timeline unfortunately.
>
> Sure, nor I; but having a goal always helps, even if it's only best-effort.
>
> Looking at redctl, it seems like actually a pretty full-featured
> toolstack -- that seems like a nice complete target to aim at. :-)

Sounds like a plan to me. :)

> I think we should make sure that things actually work.  The very
> original golang bindings I wrote to be able to control cpupools, so I
> think those functions should stay.  But I'm not sure if anyone's ever
> used ConsoleGetTty.  Like `xl.cfg` parsing, it's the sort of thing that
> *somebody* will probably want eventually; so I'm inclined to say it
> would be less cost to just test it and make sure it works than to remove
> it and re-add it when someone decides they need it.
>
> Did you have anything in particular in mind?

Okay, that's good to know. I don't have anything particular in mind, I
just wanted to pose the question. Since I wasn't around when you first
wrote the bindings, I wasn't aware of the motivations.

> I was sort of thinking what we might do is leave `xenlight` as mostly
> just a plain wrapper around the libxl C functions, as close to what
> might be generated as possible; and then have another package that would
> do something more useful.  For instance, having 'Vm' struct, which could
> be Start()ed, shut down, and so on; and which would keep track of
> if/when the domain died, &c.

That's a good point, it might be nice to create an API that is more
"Go-like" and not as tied to libxl semantically speaking.

> > I was going to ask about this. You had a vanity URL in place at one
> > point, right? Did `go get` ever work with that? In any case, pushing
> > to another repo might be desirable.
> We never actually had a URL in place, no.  I had simply chosen the URL
> based on what would be a good combination of easy to type/remember and
> easy to set up effectively.  (This was after having an informal chat
> with Ian Jackson, who tends to do most of this sort of technical admin
> thing.)  We'd probably want to go back and figure out what kind of
> "interface" is possible, how to do versioning &c, and then work
> backwards from that to get a workflow from the various Xen branches into
> that.

What do you mean by "figure out what kind of interface is possible"?
For versioning, I think it would be nice to adopt Go modules and
follow their semantic versioning recommendations. Have you considered
that route, or any others?

> BTW do you guys have a solution to the "install new tools then reboot"
> issue?  I guess if you have a daemon then it will retain the old version
> of the library until after you reboot?

At the moment, we don't have a solution to that problem as we're
simply calling xl via os/exec. But yes, when we integrate xenlight
into redctl the daemon will retain the old version until reboot.

Thanks,
-NR

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2020-02-04 19:27 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-17 15:57 [Xen-devel] [PATCH v3 1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC George Dunlap
2020-01-17 15:57 ` [Xen-devel] [PATCH v3 2/8] go/xenlight: Fix CpuidPoliclyList conversion George Dunlap
2020-01-20 23:30   ` Nick Rosbrook
2020-01-17 15:57 ` [Xen-devel] [PATCH v3 3/8] go/xenlight: More informative error messages George Dunlap
2020-01-20 23:32   ` Nick Rosbrook
2020-01-17 15:57 ` [Xen-devel] [PATCH v3 4/8] golang/xenlight: Errors are negative George Dunlap
2020-01-20 23:40   ` Nick Rosbrook
2020-01-17 15:57 ` [Xen-devel] [PATCH v3 5/8] golang/xenlight: Default loglevel to DEBUG until we get everything working George Dunlap
2020-01-20 23:41   ` Nick Rosbrook
2020-01-21  9:55     ` George Dunlap
2020-01-24 19:51       ` Nick Rosbrook
2020-01-17 15:57 ` [Xen-devel] [PATCH v3 6/8] golang/xenlight: Don't leak memory on context open failure George Dunlap
2020-01-20 23:43   ` Nick Rosbrook
2020-01-17 15:57 ` [Xen-devel] [PATCH v3 7/8] golang/xenlight: Notify xenlight of SIGCHLD George Dunlap
2020-01-17 16:52   ` Ian Jackson
2020-01-17 17:33     ` George Dunlap
2020-01-17 18:12       ` [Xen-devel] [PATCH] libxl: event: Document lifetime API for libxl_childproc_setmode Ian Jackson
2020-01-20 12:06         ` Wei Liu
2020-01-17 18:13   ` [Xen-devel] [PATCH v3 7/8] golang/xenlight: Notify xenlight of SIGCHLD Nick Rosbrook
2020-01-17 18:28     ` George Dunlap
2020-01-17 15:57 ` [Xen-devel] [PATCH v3 8/8] RFC: Sketch constructors, DomainCreateNew George Dunlap
2020-01-17 18:38   ` George Dunlap
2020-01-22 10:32   ` George Dunlap
2020-01-24 19:32   ` Nick Rosbrook
2020-01-27 18:08     ` George Dunlap
2020-01-28 20:41       ` Nick Rosbrook
2020-01-29 14:17         ` Nick Rosbrook
2020-01-29 14:46         ` George Dunlap
2020-02-04 19:26           ` Nick Rosbrook
2020-01-17 16:04 ` [Xen-devel] [PATCH v3 1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC George Dunlap
2020-01-20 23:39 ` Nick Rosbrook
2020-01-21 17:35   ` George Dunlap

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).