| Message ID | 20200210132252.381343-1-laurent@vivier.eu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | q800: fix coverity warning CID 1412799 | expand |
On 2/10/20 2:22 PM, Laurent Vivier wrote: > Check the return value of blk_write() and log an error if any > Fixes: Coverity CID 1412799 (Error handling issues) > Signed-off-by: Laurent Vivier <laurent@vivier.eu> > --- > hw/misc/mac_via.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c > index b7d0012794..81343301b1 100644 > --- a/hw/misc/mac_via.c > +++ b/hw/misc/mac_via.c > @@ -30,6 +30,7 @@ > #include "hw/qdev-properties.h" > #include "sysemu/block-backend.h" > #include "trace.h" > +#include "qemu/log.h" > > /* > * VIAs: There are two in every machine, > @@ -381,8 +382,10 @@ static void via2_irq_request(void *opaque, int irq, int level) > static void pram_update(MacVIAState *m) > { > if (m->blk) { > - blk_pwrite(m->blk, 0, m->mos6522_via1.PRAM, > - sizeof(m->mos6522_via1.PRAM), 0); > + if (blk_pwrite(m->blk, 0, m->mos6522_via1.PRAM, > + sizeof(m->mos6522_via1.PRAM), 0) < 0) { > + qemu_log("pram_update: cannot write to file\n"); > + } > } > } > >
Hi Laurent, Cc'ing qemu-block@ On 2/10/20 3:56 PM, Philippe Mathieu-Daudé wrote: > On 2/10/20 2:22 PM, Laurent Vivier wrote: >> Check the return value of blk_write() and log an error if any >> > > Fixes: Coverity CID 1412799 (Error handling issues) > >> Signed-off-by: Laurent Vivier <laurent@vivier.eu> >> --- >> hw/misc/mac_via.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c >> index b7d0012794..81343301b1 100644 >> --- a/hw/misc/mac_via.c >> +++ b/hw/misc/mac_via.c >> @@ -30,6 +30,7 @@ >> #include "hw/qdev-properties.h" >> #include "sysemu/block-backend.h" >> #include "trace.h" >> +#include "qemu/log.h" >> /* >> * VIAs: There are two in every machine, >> @@ -381,8 +382,10 @@ static void via2_irq_request(void *opaque, int >> irq, int level) >> static void pram_update(MacVIAState *m) >> { >> if (m->blk) { >> - blk_pwrite(m->blk, 0, m->mos6522_via1.PRAM, >> - sizeof(m->mos6522_via1.PRAM), 0); >> + if (blk_pwrite(m->blk, 0, m->mos6522_via1.PRAM, >> + sizeof(m->mos6522_via1.PRAM), 0) < 0) { >> + qemu_log("pram_update: cannot write to file\n"); I am not comfortable reviewing this patch, because it use a undocumented function. If I understand co-routine code enough, this eventually calls blk_co_pwritev_part(), which on success returns bdrv_co_pwritev_part(), which on success returns bdrv_aligned_pwritev(), which itself returns 0 or errno (as negative value). I don't understand how to treat the error, but apparently other devices do the same (only report some error and discarding the block not written). So this can happens if your filesystem got full, the VM is running, the device can not sync the VIA PRAM but keeps running. You let the user the possibility to make some space on the filesystem so the next sync will succeed. This does not seem critical, so I dare to add: Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> But I'd rather have one of the block folks reviewing this pattern. Regards, Phil. >> + } >> } >> } >>
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c index b7d0012794..81343301b1 100644 --- a/hw/misc/mac_via.c +++ b/hw/misc/mac_via.c @@ -30,6 +30,7 @@ #include "hw/qdev-properties.h" #include "sysemu/block-backend.h" #include "trace.h" +#include "qemu/log.h" /* * VIAs: There are two in every machine, @@ -381,8 +382,10 @@ static void via2_irq_request(void *opaque, int irq, int level) static void pram_update(MacVIAState *m) { if (m->blk) { - blk_pwrite(m->blk, 0, m->mos6522_via1.PRAM, - sizeof(m->mos6522_via1.PRAM), 0); + if (blk_pwrite(m->blk, 0, m->mos6522_via1.PRAM, + sizeof(m->mos6522_via1.PRAM), 0) < 0) { + qemu_log("pram_update: cannot write to file\n"); + } } }
Check the return value of blk_write() and log an error if any Signed-off-by: Laurent Vivier <laurent@vivier.eu> --- hw/misc/mac_via.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)