On 05/01/2023 8:15 am, Jan Beulich wrote: > On 04.01.2023 19:34, Andrew Cooper wrote: >> On 04/01/2023 5:04 pm, Jan Beulich wrote: >>> On 03.01.2023 21:09, Andrew Cooper wrote: >>>> API/ABI wise, XENVER_build_id could be merged into xenver_varstring_op(), >>>> but >>>> the internal infrastructure is awkward. >>> I guess build-id also doesn't fit the rest because of not returning strings, >>> but indeed an array of bytes. You also couldn't use strlen() on the array. >> The format is unspecified, but it is a base64 encoded ASCII string (not >> NUL terminated). > Where are you taking this base64 encoding from? I see the build ID being pure > binary data, which could easily have an embedded nul.
Oh, so it is. I'd failed to spot that libxl formats it automatically on behalf of the caller. >>>> + if ( sz > INT32_MAX ) >>>> + return -E2BIG; /* Compat guests. 2G ought to be plenty. */ >>> While the comment here and in the public header mention compat guests, >>> the check is uniform. What's the deal? >> Well its either this, or a (comat ? INT32_MAX : INT64_MAX) check, along >> with the ifdefary and predicates required to make that compile. >> >> But there's not a CPU today which can actually move 2G of data (which is >> 4G of L1d bandwidth) without suffering the watchdog (especially as we've >> just read it once for strlen(), so that's 6G of bandwidth), nor do I >> expect this to change in the forseeable future. >> >> There's some boundary (probably far lower) beyond which we can't use the >> algorithm here. >> >> There wants to be some limit, and I don't feel it is necessary to make >> it variable on the guest type. > Sure. My question was merely because of the special mentioning of 32-bit / > compat guests. I'm fine with the universal limit, and I'd also be fine > with a lower (universal) bound. All I'm after is that the (to me at least) > confusing comments be adjusted. How about 16k then? >> But overall, I'm not seeing a major objection to this change? In which >> case I'll go ahead and do the tools/ cleanup too for v2. > Well, I'm not entirely convinced of the need for the new subops (we could > as well introduce build time checks to make sure no truncation will occur > for the existing ones), but I also won't object to their introduction. At > least for the command line I can see that we will want (need) to support > more than 1k worth of a string, considering how many options we have > accumulated. I've legitimate customer bugs caused by the cmdline limit, and real test problems caused by the extraversion limit which I'm unwilling to "fix" by sticking to the current API/ABI. ~Andrew
