Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] PropertyCollector not populating ScsiLun.AlternateName data correctly/fully #3469

Closed
flamingdumpster opened this issue Jun 13, 2024 · 5 comments · Fixed by #3476
Closed

Comments

@flamingdumpster
Copy link

flamingdumpster commented Jun 13, 2024

Describe the bug

When using mo.HostSystem.Properties and specifying nil for ps, ScsiLun.AlternateName.Data is only populated with the first (least most significant) item in the json array that is stored in the vmware database. For each entry in AlternateName in the GENERIC_VPD namespace, the Data field should contain the inquiry data for the VPDs as reported available by the standard inquiry and read by vmware. Using govmomi, we can only get the least most significant entry in the array (internally represented by vmware and visible using the vm-support command to see the vmware-vimdump file.) This value will show up as 1-3 bytes, depending on the number (i.e.. 125 appears as '313235') - see output below.

To Reproduce
Steps to reproduce the behavior:

This is the code I am using to see the issue



    hosts, err := finder.HostSystemList(ctx, "*")
	if err != nil {
		return nil, err
	}

	var targetHost *object.HostSystem
	for _, h := range hosts {

		if h.Name() == hostname {
			targetHost = h
			break
		}
	}
	
	var populatedHost mo.HostSystem
	hostSystemErr := targetHost.Properties(ctx, targetHost.Reference(), nil, &populatedHost)

	if hostSystemErr != nil {
		log.Debugf("Host System Query bombed: %s", hostSystemErr.Error())
		return lunNames, hostSystemErr
	}

	hostLuns := populatedHost.Config.StorageDevice.ScsiLun

	for _, lun := range hostLuns {

		log.Debugf("** Lun: %s **", lun.GetScsiLun().CanonicalName)

		altNames := lun.GetScsiLun().AlternateName

		for _, alt := range altNames {
			log.Debugf("NS: %s, ID: %x Data: %x", alt.Namespace, alt.NamespaceId, alt.Data)
		}
	}
	

Expected behavior
for GENERIC_VPD namespace, the Data field should contain the full amount of data stored in the json array in the vmware internal database. It currently only gets the single digit, as explained above. It also appears other namespaces have the same issue.

Affected version
github.com/vmware/govmomi v0.36.2

Screenshots/Debug Output

the output appears like this when the above code is run.


time="2024-06-11T18:30:50Z" level=debug msg="** Lun: <redacted> **"
time="2024-06-11T18:30:50Z" level=debug msg="NS: GENERIC_VPD, ID: 5 Data: 2d3438"
time="2024-06-11T18:30:50Z" level=debug msg="NS: SERIALNUM, ID: 4 Data: 30"
time="2024-06-11T18:30:50Z" level=debug msg="NS: NAA, ID: 3 Data: 30"
time="2024-06-11T18:30:50Z" level=debug msg="NS: UNKNOWN, ID: 5 Data: 34"
time="2024-06-11T18:30:50Z" level=debug msg="NS: UNKNOWN, ID: 5 Data: 31"
time="2024-06-11T18:30:50Z" level=debug msg="NS: GENERIC_VPD, ID: 5 Data: 30"
time="2024-06-11T18:30:50Z" level=debug msg="NS: GENERIC_VPD, ID: 5 Data: 30"
time="2024-06-11T18:30:50Z" level=debug msg="NS: GENERIC_VPD, ID: 5 Data: 313235"
time="2024-06-11T18:30:50Z" level=debug msg="NS: GENERIC_VPD, ID: 5 Data: 313235"
time="2024-06-11T18:30:50Z" level=debug msg="NS: GENERIC_VPD, ID: 5 Data: 30"
time="2024-06-11T18:30:50Z" level=debug msg="NS: GENERIC_VPD, ID: 5 Data: 313039"
time="2024-06-11T18:30:50Z" level=debug msg="NS: GENERIC_VPD, ID: 5 Data: 313033"
time="2024-06-11T18:30:50Z" level=debug msg="NS: GENERIC_VPD, ID: 5 Data: 30"
time="2024-06-11T18:30:50Z" level=debug msg="NS: GENERIC_VPD, ID: 5 Data: 313235"
time="2024-06-11T18:30:50Z" level=debug msg="NS: GENERIC_VPD, ID: 5 Data: 313235"
time="2024-06-11T18:30:50Z" level=debug msg="NS: GENERIC_VPD, ID: 5 Data: 30"


Additional context
Using pyvmomi, we are able to see this data without an issue. Both libraries should provide the same functionality.

Copy link
Contributor

Howdy 🖐   flamingdumpster ! Thank you for your interest in this project. We value your feedback and will respond soon.

If you want to contribute to this project, please make yourself familiar with the CONTRIBUTION guidelines.

@dougm
Copy link
Member

dougm commented Jun 22, 2024

This is the same problem as #1977 , tl;dr vCenter and Go's encoding/xml package marshals []byte differently. I'm working on a fix and will share the details with that soon. Enhancing vcsim, tests and govc as part of this. Do you have something you can share that decodes AlternateName.Data? A command we can pipe to from govc would be ideal. Or if there's already some Go code available, maybe we bake something into the govc host.storage.info command.

dougm added a commit to dougm/govmomi that referenced this issue Jun 24, 2024
Go's encoding/xml package and vCenter marshal '[]byte' differently:
- Go encodes the entire array in a single xml element, for example:
  <foo>
    <bar>hello</bar>
  </foo>

- vCenter encodes each byte of the array in its own xml element, example with same data as above:
  <foo>
    <bar>104</bar>
    <bar>101</bar>
    <bar>108</bar>
    <bar>108</bar>
    <bar>111</bar>
  </foo>

This behavior is hardwired, see the xml/encoding source for the handful of []byte special cases along the lines of:

  if reflect.Slice && slice element type == reflect.Uint8

While govmomi maintains a fork of Go's xml/encoding package, we prever to further diverge.
Proposed solution is to use a 'ByteArray' type that implements xml marshalling as vCenter does,
but otherwise behaves just as '[]byte' does.

Commits that follow enhance vcsim and govc support around []byte fields.

Fixes vmware#1977
Fixes vmware#3469
dougm added a commit to dougm/govmomi that referenced this issue Jun 24, 2024
Go's encoding/xml package and vCenter marshal '[]byte' differently:
- Go encodes the entire array in a single xml element, for example:
  <foo>
    <bar>hello</bar>
  </foo>

- vCenter encodes each byte of the array in its own xml element, example with same data as above:
  <foo>
    <bar>104</bar>
    <bar>101</bar>
    <bar>108</bar>
    <bar>108</bar>
    <bar>111</bar>
  </foo>

This behavior is hardwired, see the xml/encoding source for the handful of []byte special cases along the lines of:

  if reflect.Slice && slice element type == reflect.Uint8

While govmomi maintains a fork of Go's xml/encoding package, we prefer to further diverge.
Proposed solution is to use a 'ByteArray' type that implements xml marshaling as vCenter does,
but otherwise behaves just as '[]byte' does.

Commits that follow enhance vcsim and govc support around various []byte fields.

Fixes vmware#1977
Fixes vmware#3469
dougm added a commit to dougm/govmomi that referenced this issue Jun 24, 2024
Go's encoding/xml package and vCenter marshal '[]byte' differently:
- Go encodes the entire array in a single xml element, for example:
  <foo>
    <bar>hello</bar>
  </foo>

- vCenter encodes each byte of the array in its own xml element, example with same data as above:
  <foo>
    <bar>104</bar>
    <bar>101</bar>
    <bar>108</bar>
    <bar>108</bar>
    <bar>111</bar>
  </foo>

This behavior is hardwired, see the xml/encoding source for the handful of []byte special cases along the lines of:

  if reflect.Slice && slice element type == reflect.Uint8

While govmomi maintains a fork of Go's xml/encoding package, we prefer to further diverge.
Proposed solution is to use a 'ByteArray' type that implements xml marshaling as vCenter does,
but otherwise behaves just as '[]byte' does.

Commits that follow enhance vcsim and govc support around various []byte fields.

Fixes vmware#1977
Fixes vmware#3469
dougm added a commit to dougm/govmomi that referenced this issue Jun 24, 2024
Go's encoding/xml package and vCenter marshal '[]byte' differently:
- Go encodes the entire array in a single xml element, for example:
  <foo>
    <bar>hello</bar>
  </foo>

- vCenter encodes each byte of the array in its own xml element, example with same data as above:
  <foo>
    <bar>104</bar>
    <bar>101</bar>
    <bar>108</bar>
    <bar>108</bar>
    <bar>111</bar>
  </foo>

This behavior is hardwired, see the xml/encoding source for the handful of []byte special cases along the lines of:

  if reflect.Slice && slice element type == reflect.Uint8

While govmomi maintains a fork of Go's xml/encoding package, we prefer to further diverge.
Proposed solution is to use a 'ByteArray' type that implements xml marshaling as vCenter does,
but otherwise behaves just as '[]byte' does.

Commits that follow enhance vcsim and govc support around various []byte fields.

Fixes vmware#1977
Fixes vmware#3469
@flamingdumpster
Copy link
Author

Do you have something you can share that decodes AlternateName.Data? A command we can pipe to from govc would be ideal. Or if there's already some Go code available, maybe we bake something into the govc host.storage.info command.

The AlternateName.Data field is application/vendor specific so no code that would be useful to test with. We are using govmomi inside of a plugin and not govc itself.

I guess the "definition of done" on this, from our point of view, would be that AlternateName.Data contains all of the bytes that vmware is storing for it, and not just the single entry from the json array that we see now. As long as all the data is there, in the same way it is there for pyvmomi, it should be good.

@flamingdumpster
Copy link
Author

One question - what would be the best way to work around this. I am going to look at doing a soap request directly using the govmomi client for this particular set of information, but am wondering if there is an easier way.

@dougm
Copy link
Member

dougm commented Jun 24, 2024

Change is merged and will be creating a new release today. You can test it out using govc, ended up just testing the length:

govmomi/govc/test/host.bats

Lines 163 to 172 in c1d0832

names=$(govc host.storage.info -json | jq -r .storageDeviceInfo.scsiLun[].alternateName[].data)
# given data is hex encoded []byte and:
# [0] == encoding
# [1] == type
# [2] == ?
# [3] == length
# validate name is at least 2 char x 4
for name in $names; do
[ "${#name}" -ge 8 ]
done

Example with main branch:

% govc host.storage.info -host $host -json | jq -r '.storageDeviceInfo.scsiLun[].alternateName | select(. != null) | .[].data'
TlZNZV9fX19EZWxsX0VudF9OVk1lX1A1NjAwX01VX1UuMl8xLjZUQl9fX19fX19fMDAwMzUwODMwREU0RDI1Qw==
TlZNZV9fX19EZWxsX0VudF9OVk1lX1A1NTAwX1JJX1UuMl8zLjg0VEJfX19fX19fMDAwMzRCQkYwMEU0RDI1Qw==
AAAAAwCAgw==
null
AQIACABQQwAAAAAA
AAAABQCAg7Cx
UEhZSDExOTQwMkNSMjQwSiAgICA=
AgEAREFUQSAgICAgU1NEU0NLS0IyNDBHOFIgICAgICAgICAgICAgICAgICAgICAgICAgIFBIWUgxMTk0MDJDUjI0MEogICAg
ALAAPAAAAAgCAAAAAAAAAAAAAAAA//8AAAABAAAAAAEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==
ALEAPAABAAcAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==

vs 0.37.3 release:

% ./govc host.storage.info -host $host -json | jq -r '.storageDeviceInfo.scsiLun[].alternateName | select(. != null) | .[].data'
Njc=
Njc=
LTEyNQ==
null
MA==
LTc5
MzI=
MzI=
MA==
MA==

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants