Re: Qcow2: Clean up logging/error handling

2018-11-16 Thread Reyk Floeter
On Sat, Nov 03, 2018 at 01:53:08PM -0700, Ori Bernstein wrote: > On Tue, 30 Oct 2018 23:01:50 -0700, Mike Larkin wrote: > > > On Tue, Oct 30, 2018 at 10:41:21PM -0700, o...@eigenstate.org wrote: > > > > On Tue, Oct 30, 2018 at 10:32:37PM -0700, Ori Bernstein wrote: > > > >> On Tue, 30 Oct 2018

Re: Qcow2: Clean up logging/error handling

2018-11-03 Thread Ori Bernstein
On Tue, 30 Oct 2018 23:01:50 -0700, Mike Larkin wrote: > On Tue, Oct 30, 2018 at 10:41:21PM -0700, o...@eigenstate.org wrote: > > > On Tue, Oct 30, 2018 at 10:32:37PM -0700, Ori Bernstein wrote: > > >> On Tue, 30 Oct 2018 22:29:01 -0700, Mike Larkin > > >> wrote: > > >> > > >> > > -

Re: Qcow2: Clean up logging/error handling

2018-10-31 Thread Mike Larkin
On Tue, Oct 30, 2018 at 10:41:21PM -0700, o...@eigenstate.org wrote: > > On Tue, Oct 30, 2018 at 10:32:37PM -0700, Ori Bernstein wrote: > >> On Tue, 30 Oct 2018 22:29:01 -0700, Mike Larkin > >> wrote: > >> > >> > > - if (disk->base->clustersz != disk->clustersz) { > >> > > -

Re: Qcow2: Clean up logging/error handling

2018-10-30 Thread ori
> On Tue, Oct 30, 2018 at 10:32:37PM -0700, Ori Bernstein wrote: >> On Tue, 30 Oct 2018 22:29:01 -0700, Mike Larkin wrote: >> >> > > -if (disk->base->clustersz != disk->clustersz) { >> > > -log_warn("%s: all disks must share clustersize", >> > > +

Re: Qcow2: Clean up logging/error handling

2018-10-30 Thread Mike Larkin
On Tue, Oct 30, 2018 at 10:32:37PM -0700, Ori Bernstein wrote: > On Tue, 30 Oct 2018 22:29:01 -0700, Mike Larkin wrote: > > > > - if (disk->base->clustersz != disk->clustersz) { > > > - log_warn("%s: all disks must share clustersize", > > > + fatal("%s:

Re: Qcow2: Clean up logging/error handling

2018-10-30 Thread Ori Bernstein
On Tue, 30 Oct 2018 22:29:01 -0700, Mike Larkin wrote: > > - if (disk->base->clustersz != disk->clustersz) { > > - log_warn("%s: all disks must share clustersize", > > + fatal("%s: could not open %s", basepath, __func__); > > is this right? > > > +

Re: Qcow2: Clean up logging/error handling

2018-10-30 Thread Mike Larkin
On Tue, Oct 30, 2018 at 10:01:08PM -0700, Ori Bernstein wrote: > On Sun, 28 Oct 2018 00:58:57 +0200, Reyk Floeter wrote: > > > Most of these are fatal and log_debug. Keep the __func__ there! But we___ll > > remove it from other logging functions where it was never intended to be > > used and

Re: Qcow2: Clean up logging/error handling

2018-10-30 Thread Ori Bernstein
On Sun, 28 Oct 2018 00:58:57 +0200, Reyk Floeter wrote: > Most of these are fatal and log_debug. Keep the __func__ there! But we___ll > remove it from other logging functions where it was never intended to be > used and potentially reword the warnings nicely. > > Reyk > Alright, updated

Re: Qcow2: Clean up logging/error handling

2018-10-27 Thread Mike Larkin
On Sun, Oct 28, 2018 at 12:58:57AM +0200, Reyk Floeter wrote: > Most of these are fatal and log_debug. Keep the __func__ there! But we’ll > remove it from other logging functions where it was never intended to be used > and potentially reword the warnings nicely. > > Reyk > Yep. If I added

Re: Qcow2: Clean up logging/error handling

2018-10-27 Thread Reyk Floeter
Most of these are fatal and log_debug. Keep the __func__ there! But we’ll remove it from other logging functions where it was never intended to be used and potentially reword the warnings nicely. Reyk > Am 28.10.2018 um 00:39 schrieb Ori Bernstein : > >> On Sat, 27 Oct 2018 16:15:32 -0600,

Re: Qcow2: Clean up logging/error handling

2018-10-27 Thread Theo de Raadt
> > I quite dislike when software uses __func__ to tell me what internal > > function had an error. > > > > Why should a user see an error with the string virtio_qcow2_get_base, > > qc2_open, qc2_pwrite, etc? > > > > It feels unpolished. > > > > Possibly, but it is consistent with the rest of

Re: Qcow2: Clean up logging/error handling

2018-10-27 Thread Ori Bernstein
On Sat, 27 Oct 2018 16:15:32 -0600, "Theo de Raadt" wrote: > I quite dislike when software uses __func__ to tell me what internal > function had an error. > > Why should a user see an error with the string virtio_qcow2_get_base, > qc2_open, qc2_pwrite, etc? > > It feels unpolished. >

Re: Qcow2: Clean up logging/error handling

2018-10-27 Thread Theo de Raadt
I quite dislike when software uses __func__ to tell me what internal function had an error. Why should a user see an error with the string virtio_qcow2_get_base, qc2_open, qc2_pwrite, etc? It feels unpolished.

Re: Qcow2: Clean up logging/error handling

2018-10-27 Thread Ori Bernstein
On Wed, 24 Oct 2018 16:23:29 +0800, Michael Mikonos wrote: > On Tue, Oct 23, 2018 at 09:44:24PM -0700, Ori Bernstein wrote: > > This patch turns most warnings into errors, and uses the > > appropriate fatal/fatalx so that we don't print bogus error > > strings. It also adds checks for

Re: Qcow2: Clean up logging/error handling

2018-10-24 Thread Michael Mikonos
On Tue, Oct 23, 2018 at 09:44:24PM -0700, Ori Bernstein wrote: > This patch turns most warnings into errors, and uses the > appropriate fatal/fatalx so that we don't print bogus error > strings. It also adds checks for unsupported refcount sizes > and writes that clobber the header. > > Ok?