Re: [PATCH V2] qemu: pre-create the dbus directory in qemuStateInitialize

[Ján Tomko <jtomko@xxxxxxxxxx>]

On a Wednesday in 2020, Bihong Yu wrote:
> On 2020/7/22 13:36, Pino Toscano wrote:
> > On Wednesday, 22 July 2020 04:09:24 CEST Bihong Yu wrote:
> > > +    if (virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
> > > +                     VIR_DIR_CREATE_ALLOW_EXIST) < 0) {
> > > +        virReportSystemError(errno, _("Failed to create dbus state dir %s"),
> > > +                             cfg->dbusStateDir);
> >
> > Minor notes on the message:
> > - spell "D-Bus" correctly
> > - no need to abbreviate "directory"
> > - quote the path placeholder
> > so I suggest something like:
> >   "Failed to create the D-Bus state directory '%s'"
>
> Sorry, the error message is written with reference to other contexts of qemuStateInitialize(), such as:
>
>     if (virFileMakePath(cfg->memoryBackingDir) < 0) {
>         virReportSystemError(errno, _("Failed to create memory backing dir %s"),
>                              cfg->memoryBackingDir);
>         goto error;
>     }
>     if (virFileMakePath(cfg->slirpStateDir) < 0) {
>         virReportSystemError(errno, _("Failed to create slirp state dir %s"),
>                              cfg->slirpStateDir);
>         goto error;
>     }
>
> +    if (virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
> +                     VIR_DIR_CREATE_ALLOW_EXIST) < 0) {
> +        virReportSystemError(errno, _("Failed to create dbus state dir %s"),
> +                             cfg->dbusStateDir);
> +        goto error;
> +    }
>
>
> So I don't think that's a good suggestion. If you still insist your suggestion, I will rewrite it.

They are good suggestions but I'm afraid they're out of scope of
this patch.

Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx>

Jano

Attachment: signature.asc
Description: PGP signature