On 03/06/2017 02:14 PM, Max Reitz wrote: >>> if (S_ISREG(st.st_mode)) { >>> if (ftruncate(s->fd, offset) < 0) { >>> + /* The generic error message will be fine */ >>> return -errno; >> >> Relying on a generic error message in the caller is awkward. I see it as >> evidence of a partial conversion if we have an interface that requires a >> return of a negative errno value to make up for when errp is not set. > > I'm not sure what you mean by this exactly.
The ideal conversion would be having .bdrv_truncate switch to a void return; then no caller is messing with negative errno values (especially since some of them are just synthesizing errno values, rather than actually encountering one, and since some of them are probably using -1 when they should be using errno). But such a conversion requires that all drivers be updated to properly set errp. > I can agree that it's not > nice within a single driver. But I think it's fine to have some drivers > that generate an Error object and others that do not, as long as the > generic interface (bdrv_truncate()) masks this behavior. I agree that as an interim thing, having some drivers that aren't converted yet is the best way to make progress. But it's still best if every driver is switched on an all-or-none basis, so that we don't forget to go back and re-fix drivers that half-heartedly set errp if we eventually reach the point where all other drivers have been fixed. In other words, I view the generic bdrv_truncate() supplying an error based on negative errno is a crutch, and not something that we should rely on, at least not for the drivers that we fix to set errp directly. > >> I >> know you aren't comfortable converting all drivers, but for the drivers >> you do convert, I'd rather guarantee that ALL errors set errp. > > Can do, I just felt bad that it be would exactly the same as the message > that would be generated in bdrv_truncate() anyway. There may be some duplication of error messages, but it still seems cleaner to consistently set the error within the driver than to rely on the generic crutch. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature