> -----Original Message----- > From: Christian Schoenebeck <qemu_...@crudebyte.com> > Sent: 2022年5月5日 19:44 > To: qemu-devel@nongnu.org; Shi, Guohuai <guohuai....@windriver.com>; Greg Kurz > <gr...@kaod.org> > Cc: Meng, Bin <bin.m...@windriver.com>; Bin Meng <bmeng...@gmail.com> > Subject: Re: [PATCH 5/9] hw/9pfs: Add a 'local' file system backend driver for > Windows > > [Please note: This e-mail is from an EXTERNAL e-mail address] > > On Mittwoch, 4. Mai 2022 21:34:22 CEST Shi, Guohuai wrote: > [...] > > > > index 0000000000..aab7c9f7b5 > > > > --- /dev/null > > > > +++ b/hw/9pfs/9p-local-win32.c > > > > @@ -0,0 +1,1242 @@ > > > > +/* > > > > + * 9p Windows callback > > > > + * > > > > + * Copyright (c) 2022 Wind River Systems, Inc. > > > > + * > > > > + * Based on hw/9pfs/9p-local.c > > > > + * > > > > + * This work is licensed under the terms of the GNU GPL, version 2. > > > > See > > > > + * the COPYING file in the top-level directory. > > > > + */ > > > > + > > > > +/* > > > > + * Not so fast! You might want to read the 9p developer docs first: > > > > + * https://wiki.qemu.org/Documentation/9p > > > > + */ > > > > + > > > > +#include "qemu/osdep.h" > > > > +#include <windows.h> > > > > +#include <dirent.h> > > > > +#include "9p.h" > > > > +#include "9p-local.h" > > > > +#include "9p-xattr.h" > > > > +#include "9p-util.h" > > > > +#include "fsdev/qemu-fsdev.h" /* local_ops */ > > > > +#include "qapi/error.h" > > > > +#include "qemu/cutils.h" > > > > +#include "qemu/error-report.h" > > > > +#include "qemu/option.h" > > > > +#include <libgen.h> > > > > > > I'm not sure whether all of this (i.e. 9p-local-win32.c in general) > > > is really needed. I mean yes, it probably does the job, but you > > > basically add a complete separate 'local' backend implementation > > > just for the Windows host platform. > > > > > > My expectation would be to stick to 9p-local.c being OS-agnostic, > > > and only define OS-specific functionality in 9p-util-windows.c if needed. > > > > > > And most importantly: don't duplicate code as far as possible! I > > > mean somebody would need to also review and maintain all of this. > > > > Actually, almost all FileOperations functions are re-written for > > Windows host. > > > > Here is the comparison statistics for 9p-local.c and 9p-local-win32.c: > > Total lines : 1611 > > Changed lines: 590 > > Inserted lines: 138 > > Removed lines: 414 > > > > For function level difference count: > > > > Changed function: 39 > > Unchanged function: 13 > > > > If use "#ifdef _WIN32" to sperate Windows host code, it may need to be > > inserted about 150 code blocks (or 39 code blocks for all changed > > functions). > > > > I am not sure if it is a good idea to insert so many "#ifdef _WIN32", > > it may cause this file not readable. > > > > If stick to 9p-local.c being OS-agnostic, I think it is better to > > create two new files: 9p-local-linux.c and 9p-local-win32.c > > The thing is, as this is presented right now, I can hardly even see where > deviating > behaviour for Windows would be, where not, and I'm missing any explanations > and > reasons for the individual deviations right now. Chances are that you are > unnecessarilly adding duplicate code and unnecessary code deviations. For me > this > 9p-local-win32.c approach looks overly complex and not appropriately > abstracted > on first sight. > > I suggest waiting for Greg to give his opinion on this as well before > continuing. > > Best regards, > Christian Schoenebeck >
I can make this commit to be split into several commits: Commit #1: only make Windows host built successfully. Commit #2: provide some basic file system operations and leave other functions not workable (return -1). Commit #3: provide full file system operations. Commit #4: provide security mode "mapped" by NTFS ADS Commit #5: provide security mode "mapped-file" Commit #6: fix Windows (MinGW) directory access API compatible issues However, I think I may still need a standalone file "9p-local-win32.c". Some functions could be moved (or merged) back to 9p-local.c. But there are still some functions are too complex to be merged. Thanks Guohuai