Re: [PATCH crosvm v2] crosvm: support setting guest MAC from tap-fd

Alyssa Ross
2021-05-29T14:08:56Z

[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]

"Cole Helbling" <cole.e.helbling@outlook.com> writes:

> On Tue May 18, 2021 at 1:36 AM PDT, Alyssa Ross wrote:
>> "Cole Helbling" <cole.e.helbling@outlook.com> writes:
>> > Is there a better Error variant for this? `TooManyArguments` seems not-
>> > completely-accurate when specifying an already-in-use FD.
>>
>> Here are all the variants. Do you think there's a better one?
>>
>> https://chromium.googlesource.com/chromiumos/platform/crosvm/+/f35d2c43ff19520855cffee761dc8899c5a439a1/src/argument.rs#49
>>
>> The only other one I can see that might be applicable would be
>> InvalidValue...
>
> Yeah, I think InvalidValue would be a better choice (IMHO). The user hasn't
> provided too many arguments; they just provided a invalid TAP FD because it's
> already in use.

Hmm, I agree with you, but I just went to make this change, and noticed
that everywhere else in the file, they use TooManyArguments for this
sort of case. e.g. "wayland socket name already used". So I think it's
probably best to stick with TooManyArguments for consistency (and that's
probably why I did it like that in the first place, although I don't
remember).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]