[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] network: Fix upgrade from libvirt older than 1.2.4



On 11/26/2014 09:07 AM, Jiri Denemark wrote:
> Starting from libvirt-1.2.4, network state XML files moved to another
> directory (see commit b9e95491) and libvirt automatically migrates the
> network state files to a new location. However, the code used
> dirent.dt_type which is not supported by all filesystems. Thus, when
> libvirt is upgraded on a host which uses such filesystem, network state
> XMLs were not properly moved and running networks disappeared from
> libvirt.
> 
> This patch falls back to stat() whenever dirent.dt_type is DT_UNKNOWN to

s/dt_type/d_type/

> fix this issue.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1167145
> Signed-off-by: Jiri Denemark <jdenemar redhat com>
> ---
>  src/network/bridge_driver.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)

> 
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 6cb421c..7066dfe 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -499,15 +499,34 @@ networkMigrateStateFiles(void)
>      }
>  
>      while ((direrr = virDirRead(dir, &entry, oldStateDir)) > 0) {
> +        if (entry->d_type != DT_UNKNOWN &&
> +            entry->d_type != DT_REG)

Even the existence of a d_type member is an extension that is not
guaranteed to compile.  If someone complains, we may have to introduce a
helper macro (gnulib fts.c uses a macro D_TYPE(d) that resolves to
(d)->d_type where supported, and to DT_UNKNOWN elsewhere; although that
particular file is not LGPLv2 so we don't use it in libvirt).  But as
your patch isn't changing whether something would compile, you don't
need to make that change now.

> +            continue;
>  
> -        if (entry->d_type != DT_REG ||
> -            STREQ(entry->d_name, ".") ||
> +        if (STREQ(entry->d_name, ".") ||
>              STREQ(entry->d_name, ".."))
>              continue;
>  
>          if (virAsprintf(&oldPath, "%s/%s",
>                          oldStateDir, entry->d_name) < 0)
>              goto cleanup;
> +
> +        if (entry->d_type == DT_UNKNOWN) {
> +            struct stat st;
> +
> +            if (stat(oldPath, &st) < 0) {

Shouldn't this be lstat() instead of stat(), so we aren't chasing symlinks?

ACK with that answered.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]